-
Notifications
You must be signed in to change notification settings - Fork 5
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
GCS migration updates #87
Conversation
5f68c5b
to
fd5d803
Compare
fd5d803
to
04e80dc
Compare
app/telemetry/handler.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is used in a google cloud function, so adjusting it to have a configured network id I think would require changes on those separate deploy instructions. I think the default value on getting it if the environment variable doesn't exist would make it continue doing the same thing, but I'd also be fine with leaving this file unchanged since the proxy server itself doesn't use it.
https://github.com/Pocket/proxy-server/blob/main/README.md#telemetry-function are the directions that reference this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know. Did the change here mostly to make tests happy. The NETWORK_ID
var was originally hard-wired, it would still be the same now, but read ADZERK_NETWORK_ID
if present, so shouldn't change the actual situation a lot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anything outstanding here? lgtm otherwise 😄
No description provided.