Gcs database #12817

bkochendorfer · 2023-02-27T22:35:03Z

One-line summary

Allow uploading and downloading from GCP CS (GCS) instead of AWS S3.

Significant changes and points to review

I felt like just throwing an error during the upload if there was a failure is fine but let me know if you want me to try to figure out what to catch.

Issue / Bugzilla link

#12637

Testing

I deployed the change manually in our dev gcp environment and connected to the clock pod to manually run an upload and a download.

…r more than s3

robhudson · 2023-02-28T17:56:28Z

bin/run-db-download.py

@@ -21,9 +21,12 @@
 BUCKET_NAME = os.getenv("AWS_DB_S3_BUCKET", "bedrock-db-dev")
 REGION_NAME = os.getenv("AWS_DB_REGION", "us-west-2")
 S3_BASE_URL = f"https://s3-{REGION_NAME}.amazonaws.com/{BUCKET_NAME}"
+GCS_BASE_URL = f"https://storage.googleapis.com/{BUCKET_NAME}"
+DOWNLOAD_FROM_GCS = os.getenv("DOWNLOAD_FROM_GCS", 'False').lower() in ('true', '1', 't')


Our tests don't check black formatting on these files, but it would be nice if they passed (wrt the single vs double quoting).

Sounds good, this line might be a little over the top too. Open to feedback on how you all like to handle boolean values as env variables.

In other parts of the code base we use everett. If we used it here this could be written as DOWNLOAD_FROM_GCS = config("DOWNLOAD_FROM_GCS", parser=bool, default="false"). But not sure why it isn't already used here.

oh cool, thanks @robhudson pushed up a change that pulls that in.

Welp, maybe that is why looks to fail the tests:

Traceback (most recent call last): File "/app/./bin/run-db-download.py", line 10, in <module> from bedrock.base.config_manager import config ModuleNotFoundError: No module named 'bedrock'

robhudson · 2023-03-01T21:43:51Z

Looks like ./bin/cron.py imports it but there's a path that is added in that one for it to work.

bkochendorfer · 2023-03-01T22:08:29Z

Thanks @robhudson !

robhudson · 2023-03-01T22:58:32Z

bin/run-db-download.py

+
+# must import after adding bedrock to path
+from bedrock.base.config_manager import config  # noqa
+
 BUCKET_NAME = os.getenv("AWS_DB_S3_BUCKET", "bedrock-db-dev")
 REGION_NAME = os.getenv("AWS_DB_REGION", "us-west-2")


Since we have config now, we could optionally convert these (or anywhere else in this file where we use os.getenv). An example of a string value would just be:

BUCKET_NAME = config("AWS_DB_S3_BUCKET", default="bedrock-db-dev")

Sorry to tack on so much extra.

No problem at all!

codecov · 2023-03-02T15:44:44Z

Codecov Report

Patch coverage has no change and project coverage change: -0.12 ⚠️

Comparison is base (a9a2f04) 76.99% compared to head (b7ca03a) 76.88%.

❗ Current head b7ca03a differs from pull request most recent head c3b0cc5. Consider uploading reports for the commit c3b0cc5 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12817      +/-   ##
==========================================
- Coverage   76.99%   76.88%   -0.12%     
==========================================
  Files         144      144              
  Lines        7704     7669      -35     
==========================================
- Hits         5932     5896      -36     
- Misses       1772     1773       +1

see 8 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

robhudson · 2023-03-02T15:49:30Z

bin/run-db-upload.py

@@ -63,6 +85,18 @@ def upload_db_data(db_data):
    except Boto3Error:
        return f"ERROR: Failed to upload the new database info file: {db_data}"



Do we want anything around the if not s3 line above, so that in the future if we remove the AWS credentials this won't short circuit and never make it this far? Or would changes around S3 come later?

My idea was to do kind of a multi step deploy:

Get the code out

Turn on GCS so it uploads to both GCS and S3, let that sit for a while

Turn on download from GCS, let that sit

Come back and remove the s3 references

Yep, sounds good to me.

@bkochendorfer @pmac Can we get this rolled out to prod? Good to do ahead of the postgres/cms tango

robhudson

Thanks for all the extra changes. 👍

stevejalim · 2023-06-06T21:13:10Z

@bkochendorfer Do you need one of us to merge this?

bkochendorfer · 2023-06-06T21:19:34Z

Yes I do not have merge capabilities here

robhudson · 2023-10-07T00:19:23Z

Chatted with Brett. We're going to sit on this until there's a bit more time to focus on it soon.

bkochendorfer added 2 commits February 27, 2023 13:51

chore(db_s3_utils): Rename file to db_utils as we want to use them fo…

5d7ffa4

…r more than s3

feature(gcs): Upload to GCS

950bd1a

ahoneiser approved these changes Feb 28, 2023

View reviewed changes

robhudson reviewed Feb 28, 2023

View reviewed changes

bkochendorfer added 2 commits February 28, 2023 16:07

chore: Run black on script

bcb41e0

Use everett Bedrock config for parsing Boolean

61732af

Add path so we can import config

5cfcb44

bkochendorfer added 2 commits March 1, 2023 16:21

fixup: test and lint feedback

1b9a20c

appease black

ff4e7f1

bkochendorfer requested a review from robhudson March 1, 2023 22:40

robhudson reviewed Mar 1, 2023

View reviewed changes

bkochendorfer added 2 commits March 2, 2023 09:32

run isort

8065eee

Use config on other variables

b7ca03a

robhudson reviewed Mar 2, 2023

View reviewed changes

robhudson approved these changes Mar 2, 2023

View reviewed changes

Merge branch 'main' into gcs-database

c3b0cc5

robhudson added the Do Not Merge ⚠️ label Oct 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gcs database #12817

Gcs database #12817

bkochendorfer commented Feb 27, 2023

robhudson Feb 28, 2023

bkochendorfer Feb 28, 2023

robhudson Mar 1, 2023

bkochendorfer Mar 1, 2023

bkochendorfer Mar 1, 2023

robhudson commented Mar 1, 2023

bkochendorfer commented Mar 1, 2023

robhudson Mar 1, 2023

bkochendorfer Mar 2, 2023

codecov bot commented Mar 2, 2023 •

edited

robhudson Mar 2, 2023

bkochendorfer Mar 2, 2023

robhudson Mar 2, 2023

stevejalim May 9, 2024

robhudson left a comment

stevejalim commented Jun 6, 2023

bkochendorfer commented Jun 6, 2023

robhudson commented Oct 7, 2023

		@@ -63,6 +85,18 @@ def upload_db_data(db_data):
		except Boto3Error:
		return f"ERROR: Failed to upload the new database info file: {db_data}"

Gcs database #12817

Are you sure you want to change the base?

Gcs database #12817

Conversation

bkochendorfer commented Feb 27, 2023

One-line summary

Significant changes and points to review

Issue / Bugzilla link

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robhudson commented Mar 1, 2023

bkochendorfer commented Mar 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 2, 2023 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robhudson left a comment

Choose a reason for hiding this comment

stevejalim commented Jun 6, 2023

bkochendorfer commented Jun 6, 2023

robhudson commented Oct 7, 2023

codecov bot commented Mar 2, 2023 •

edited