Dynamically extract the download URL from the feed #47
Conversation
Extract the download link.
Add depenency
ctyparser/bigcty.py
Outdated
| dl_url = f'http://www.country-files.com/bigcty/download/{update_date[:4]}/bigcty-{update_date}.zip' # TODO: Issue #10 | ||
| page = session.get(update_url) | ||
| tree = html.fromstring(page.content) | ||
| dl_url = tree.xpath("//a[contains(@href,'zip')]/@href")[0] | ||
| rq = session.get(dl_url) | ||
| if rq.status_code == 404: |
There was a problem hiding this comment.
is this codepath still necessary? I think it exists because there was a change in the feed bafa05d
There was a problem hiding this comment.
I think that fallback is needed, but very differently, and only if the download URL extraction fails (see other comment)
ctyparser/bigcty.py
Outdated
| dl_url = f'http://www.country-files.com/bigcty/download/{update_date[:4]}/bigcty-{update_date}.zip' # TODO: Issue #10 | ||
| page = session.get(update_url) | ||
| tree = html.fromstring(page.content) | ||
| dl_url = tree.xpath("//a[contains(@href,'zip')]/@href")[0] |
There was a problem hiding this comment.
what if tree.xpath() doesn't find anything? would [0] IndexError? we should handle that, perhaps
| @@ -172,7 +173,9 @@ def update(self) -> bool: | |||
|
|
|||
| with tempfile.TemporaryDirectory() as temp: | |||
| path = pathlib.PurePath(temp) | |||
| dl_url = f'http://www.country-files.com/bigcty/download/{update_date[:4]}/bigcty-{update_date}.zip' # TODO: Issue #10 | |||
| page = session.get(update_url) | |||
There was a problem hiding this comment.
should probably check for status before trying to parse the content
0x5c
changed the title
| @@ -172,7 +173,9 @@ def update(self) -> bool: | |||
|
|
|||
| with tempfile.TemporaryDirectory() as temp: | |||
| path = pathlib.PurePath(temp) | |||
| dl_url = f'http://www.country-files.com/bigcty/download/{update_date[:4]}/bigcty-{update_date}.zip' # TODO: Issue #10 | |||
There was a problem hiding this comment.
It would probably be good to keep that URL as a fallback, like the other URL format already is
ctyparser/bigcty.py
Outdated
| dl_url = f'http://www.country-files.com/bigcty/download/{update_date[:4]}/bigcty-{update_date}.zip' # TODO: Issue #10 | ||
| page = session.get(update_url) | ||
| tree = html.fromstring(page.content) | ||
| dl_url = tree.xpath("//a[contains(@href,'zip')]/@href")[0] | ||
| rq = session.get(dl_url) | ||
| if rq.status_code == 404: |
There was a problem hiding this comment.
I think that fallback is needed, but very differently, and only if the download URL extraction fails (see other comment)
| @@ -7,3 +7,4 @@ sphinx | |||
| # Dependencies | |||
| feedparser | |||
| requests | |||
| lxml | |||
There was a problem hiding this comment.
Is lxml not needed as a normal dependency? This file is only used for development, so lxml should also be added to setup.py (setup.py being what defines dependencies to install when someone install ctyparser)
|
Sorry, I think I'm creating more of a problem for you than what I'm solving. |
Description
Use lxml xpath to pull download link off of web page to clear issue #10
Fixes #10
Type of change
How has this been tested?
By running it.
Checklist
DEVELOPING.md, if it exists)CHANGELOG.mdupdated if needed