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

Don't require credentials in DB client #844

Closed
Mr0grog opened this issue Feb 9, 2023 · 0 comments · Fixed by #845
Closed

Don't require credentials in DB client #844

Mr0grog opened this issue Feb 9, 2023 · 0 comments · Fixed by #845

Comments

@Mr0grog
Copy link
Member

Mr0grog commented Feb 9, 2023

The web-monitoring-db client in web_monitoring/db.py currently requires credentials, but the HTTP API no longer requires authentication for some operations (edgi-govdata-archiving/web-monitoring-db#1069). Accordingly, the client here shouldn’t require an e-mail and password in order to use.

  • Make email and password constructor parameters optional.
  • Raise if only one of email and password has a value.
  • Don’t set auth credentials if email and password are not present.
  • Don’t require any of the env vars to be present in .from_env().
  • Consider adding a separate exception for 403 errors (we have one for 401 — not authenticated — but not 403 — forbidden/not authorized). Users without credentials will probably encounter 403 errors more often than 401 now.
Mr0grog added a commit that referenced this issue Feb 10, 2023
The web-monitoring-db API no longer requires authentication for read-only access to most objects, so the API client here shouldn't require users to set any credentials. Fixes #844.
Mr0grog added a commit that referenced this issue Feb 10, 2023
The web-monitoring-db API no longer requires authentication for read-only access to most objects, so the API client here shouldn't require users to set any credentials. Fixes #844.
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 a pull request may close this issue.

1 participant