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

PB-617 Introduce parallel bucket setup #423

Merged
merged 27 commits into from
Jul 9, 2024
Merged

Conversation

schtibe
Copy link
Contributor

@schtibe schtibe commented Jun 20, 2024

Dual bucket setup

These changes allow for having two buckets in service-stac. Some of the uploads will go to the "managed" bucket based on the file patterns.
The changes concern the multipart upload. Based on the collection, different presigned urls will be generated from the buckets.

Other things included in this PR:

  • Fixed a deprecation warning for the minio container
  • Fixed the pylint configuration
  • Removed various code that I think is not needed anymore (it's not called anywhere in the code)
  • Switch to docker compose plugin (instead of docker-compose)

@schtibe schtibe force-pushed the feat-pb-617-parallel-buckets branch from 37b7394 to 4814b05 Compare June 20, 2024 14:59
@schtibe schtibe force-pushed the feat-pb-617-parallel-buckets branch 24 times, most recently from 8c48f72 to 19449d3 Compare June 24, 2024 12:08
app/config/settings_dev.py Outdated Show resolved Hide resolved
app/config/settings_prod.py Outdated Show resolved Hide resolved
@schtibe schtibe force-pushed the feat-pb-617-parallel-buckets branch from 19449d3 to 9d5bea6 Compare June 24, 2024 12:11
schtibe added 22 commits July 8, 2024 15:15
These patterns have to go to the managed s3 bucket instead of the
existing (legacy) one
The variables weren't pointing to the correct settings, thus the pylint
in vscode wasn't properly working
The env wasn't loaded properly in pylint and thus some of the
configuration couldn't be found. This lead to errors the vscode
execution of pylint, and thus the errros weren't available in the editor
Parameterized is needed for parametrizing unit tests.
Test whether it selected the correct bucket based on the given patterns
The parallel bucket tests don't touch the API, therefore it doesn't make
sense to have them in either tests_10 or tests_09
We need django-environ for parsing the list for the managed_patterns.
Since we have this library, we can safely drop our own implementation
that parses strings from the environment and use django-environ
Have the means to provide a list of (regex) patterns in the environment
for selecting the correct s3 bucket
Have an extendable dict for the configuration of the buckets. This
simplifies the settings access and makes it more versatile
Split the boto3 storage implementation into two separate versions in
order to target the correct s3 bucket
Introduce new file field which can dynamically select the storage of the
file based on the collection's name
Instead of having to use assume_role explicitly we use the proper way of
accessing the bucket via service account
Improve the names of the settings. Make them more unified (everything
connected to s3 starts with S3_).
@schtibe schtibe force-pushed the feat-pb-617-parallel-buckets branch 3 times, most recently from 4032e37 to 22d2f8d Compare July 8, 2024 16:13
It's impossible to reproduce the service-account situation for minio,
hence we access it the legacy way with access key
@schtibe schtibe force-pushed the feat-pb-617-parallel-buckets branch from 22d2f8d to 60cc43c Compare July 9, 2024 06:50
Copy link
Contributor

@boecklic boecklic left a comment

Choose a reason for hiding this comment

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

🪣 🪣 🎉
(approved after intense personal discussion)

@schtibe schtibe merged commit 10f145a into develop Jul 9, 2024
3 checks passed
@schtibe schtibe deleted the feat-pb-617-parallel-buckets branch July 9, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants