Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Epic - New Website Property Backfill #208

Merged
merged 11 commits into from Nov 4, 2021

Conversation

BenjaminatEpic
Copy link
Contributor

Backfilling website property for endpoints from Epic submitted prior to the introduction of the property.

BenjaminatEpic and others added 2 commits October 20, 2021 20:29
Backfilling website property for endpoints from Epic submitted prior to the introduction of the property.
@jdkizer9
Copy link
Contributor

@BenjaminatEpic Seeing a failure for https://www.wellspan.org/:

RemoteProtocolError: multiple Transfer-Encoding headers

I'm not sure that it's a WellSpan issue, as it renders in the browser just fine. For some context, it appears to be the same issue as this. The requests library works just fine and the response headers are:

{'Cache-Control': 'private, no-cache', 'X-Via-NSCOPI': '1.0', 'Transfer-Encoding': 'chunked, chunked', 'Content-Type': 'text/html; charset=utf-8', 'X-UA-Compatible': 'IE=Edge', 'X-Content-Type-Options': 'nosniff', 'X-XSS-Protection': '1;mode=block', 'Content-Security-Policy': "frame-ancestors 'self' '*.wellspan.org'", 'X-Frame-Options': 'ALLOW-FROM https://my.wellspan.org', 'Date': 'Thu, 21 Oct 2021 16:49:19 GMT', 'ntCoent-Length': '84739', 'Set-Cookie': 'NSC_ESNS=0946bdec-9a40-1171-9678-bad1b267051a_3274532451_3735375718_00000000004449799910; Path=/; Expires=Thu, 21-Oct-2021 16:49:34 GMT, NSC_mc_wtws_XfmmTqbo=ffffffffc3a046f445525d5f4f58455e445a4a42378b;Version=1;path=/;secure;httponly', 'Strict-Transport-Security': 'max-age=157680000; includeSubDomains', 'Content-Encoding': 'gzip'}

I think the best solution here is to fall back to using the requests library if we encounter a protocol exception. I'll update the scripts to account for this case, and once that merges, you'll likely need to merge in / rebase your branch before this will pass.

@BenjaminatEpic
Copy link
Contributor Author

@jdkizer9 I've removed Wellspan from this PR so we can get it through while you make the changes to the script. I'll do a new PR when the script changes are made.

Copy link
Contributor

@jdkizer9 jdkizer9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Thanks @BenjaminatEpic

@jdkizer9 jdkizer9 merged commit 299c6b2 into the-commons-project:main Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants