-
Notifications
You must be signed in to change notification settings - Fork 10
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
bug-1908018: drop support for AWS #1041
Conversation
13bff04
to
bf11540
Compare
I wrote up a bug 1908646 to move the contents of that spec to a Google Doc. It's awkward to maintain it over time since it defines a project specification for a specific point in time. |
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.
Looks good!
I wondered about sentry_sdk integrations, but this PR doesn't block on that.
@@ -20,7 +20,6 @@ | |||
from fillmore.scrubber import Scrubber, Rule, SCRUB_RULES_DEFAULT | |||
import sentry_sdk | |||
from sentry_sdk.integrations.atexit import AtexitIntegration | |||
from sentry_sdk.integrations.boto3 import Boto3Integration |
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.
Can you check the sentry_sdk integrations for a GCS integration or something else that might capture GCS library things?
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.
there is gcp integration! added per https://docs.sentry.io/platforms/python/integrations/gcp-functions/
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.
nevermind, that is only for cloud functions and only works on python3.7
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.
I believe there aren't sentry integrations for the google cloud sdk
@@ -64,11 +64,11 @@ Environments | |||
Will this run in Heroku? Probably, but you'll need to do some Heroku footwork to | |||
set it up. | |||
|
|||
Will this run on AWS? Yes--that's what we do. | |||
Will this run on GCP? Yes--that's what we do. |
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.
I should do a pass on removing as much of the Antenna docs as possible. There's a lot of stuff in here to help other companies/people run Antenna and we don't support that.
@@ -194,12 +183,12 @@ Here are some good ones: | |||
|
|||
* ``breakpad_resource.crash_save.time`` | |||
|
|||
Timing. This is the time it took to save the crash to S3. | |||
Timing. This is the time it took to save the crash to Google Cloud Storage. |
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 section is bunk. I want to update Antenna to Markus 5.0.0 which includes a filter that allows us to specify all the metrics in a file, enforce that only those are emitted, and then auto-document them. I'll get to that eventually. Here's the equivalent in Eliot:
https://mozilla-eliot.readthedocs.io/en/latest/metrics.html
https://github.com/mozilla-services/eliot/blob/main/eliot/eliot_metrics.yaml
@@ -132,24 +129,6 @@ def test_foo(client, tmpdir): | |||
return AntennaTestClient(get_app(AntennaTestClient.build_config())) | |||
|
|||
|
|||
@pytest.fixture | |||
def s3mock(): |
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.
💯
2800e5b
to
cf11259
Compare
cf11259
to
7656543
Compare
specifically remove support for S3 and SQS, cloud-specific test markers, "cloud" in
/__version__
, and update docs to reference gcs/pubsub instead, exceptdocs/spec_v1.rst
which was left untouched.For testing i ran:
make build make test make setup make run
and in a different shell
make shell systemtest/test_env.sh local
to make sure the dev environment is correctly configured, and I think the rest is covered by CI.