fix issue # 643 #645
Closed
fix issue # 643 #645
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
EddyIonescu
reviewed
May 15, 2020
There was a problem hiding this comment.
Thanks for making the PR and sorry for the delay in reviewing it; I'll be sure to be more responsive over the next few days (also since it's a long-weekend) for reviewing your changes.
So there are two issues:
- While it downloads the feed, it doesn't find the feed, which was part of the ticket. It's fine if you want to leave it as a TODO for another PR. For example, in backend/agencies you'd add a "transitfeed_url" to the yaml file of each agency, then you'd make an API call to that URL as to get the feed versions.
- The goal isn't to always get the latest version but to get the latest version as of a given date. For example, if the date given is 2020-04-01 and there are GTFS feeds from 2020-05-01, 2020-03-01, and 2020-01-01, we'd want the one from 2020-03-01. Same as above, it's fine if you want to leave it as a TODO for another PR.
From the issue description, the start function should take in two arguments: an agency ID (name of the agency that corresponds to the yaml files in backend/agencies, like ttc or muni).
| :param transitfeed_key: transitfeeds API key | ||
| :param feedID: feed ID from transitfeed | ||
| :param S3bucket_name: S3 bucket name | ||
| :return: True if file was uploaded, else False |
There was a problem hiding this comment.
@EddyIonescu could you set this as another PR?
| :param feed: the unique ID of the feed | ||
| :return: filename after download | ||
| """ | ||
| replacefeed = feed.replace("/", "%2F") |
There was a problem hiding this comment.
You don't use feed after this, so you could just assign the result to feed instead of adding another variable name.
|
@EddyIonescu, could you make point 1 and 2 in your comment as another PR? I will get back to it when I have time if no one else is interested. |
hathix
requested changes
May 28, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #643