[FDS-2127] Update URL validation to use requests.options to verify connectivity #1469
Conversation
| @@ -2,17 +2,28 @@ | |||
| [](https://actions-badge.atrox.dev/Sage-Bionetworks/schematic/goto?ref=develop) [](https://sage-schematic.readthedocs.io/en/develop/?badge=develop) [](https://badge.fury.io/py/schematicpy) | |||
|
|
|||
| # Table of contents | |||
| - [Schematic](#schematic) | |||
There was a problem hiding this comment.
I have a markdown plugin in VSCODE that automatically creates/updates these. If you don't wish to have this update I will delete it from the PR.
| @@ -142,3 +142,9 @@ def temporary_file_copy(request, helpers: Helpers) -> Generator[str, None, None] | |||
| # Teardown | |||
| if os.path.exists(temp_csv_path): | |||
| os.remove(temp_csv_path) | |||
|
|
|||
|
|
|||
| @pytest.fixture(name="dmge", scope="function") | |||
There was a problem hiding this comment.
I moved this up to conftest so I could use it in my new tests as well
| @@ -0,0 +1,73 @@ | |||
| import pandas as pd | |||
There was a problem hiding this comment.
This is the start of moving some files into an tests/integration directory. We can start slow and add new items into the appropriate locations while we slowly move items over in a strangler fig pattern: https://shopify.engineering/refactoring-legacy-code-strangler-fig-pattern
There was a problem hiding this comment.
I appreciate the additive nature of these changes
There was a problem hiding this comment.
Thanks Bryan! I like the change.
|
| valid_url = True | ||
| response_code = response.getcode() | ||
| response = requests.options(url, allow_redirects=True) |
There was a problem hiding this comment.
For the FAIR team, was there a specific reason to use urllib other than the fact that it is a builtin package?
I think something like this works too
from urllib import request
req = request.Request(url, method='OPTIONS')
response = request.urlopen(req)
print(response.status, response.headers)
note: I prefer using requests package, but thought I would ask
There was a problem hiding this comment.
I see. I also prefer using requests package. What Brian has there seems perfect to me.
|
@thomasyu888 @linglp I moved this to a non-forked branch/PR: #1472 |
Problem:
https://doi.org/10.1158/0008-5472.can-23-0128were being rejected as theurlopenwas not connecting to the server hosting this webpage.Solution:
requests.optionsthat will verify the webserver in question is reachable. Read more about the options method here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/OPTIONSTesting: