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

Multiple buckets initial work #229

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

ThorodanBrom
Copy link
Collaborator

  • NOTE, existing process code needs some work + planning in order to support multiple buckets properly
    • These changes will not pass the Jenkins pipeline as a result

@ThorodanBrom ThorodanBrom force-pushed the multiple-buckets-init branch 2 times, most recently from 965cec0 to 56c9672 Compare February 5, 2025 06:01
@ThorodanBrom ThorodanBrom force-pushed the multiple-buckets-init branch 2 times, most recently from bc03412 to dffb943 Compare February 18, 2025 11:11
- Simplify S3Config class to only hold the S3 config and not do anything else
	- Add read access config option as well
- New S3ConfigsHolder class can load configs from server config, do validation and then return a config
as an S3Config object by searching for an identifier
- Make both above classes Vert.x data objects
- Add S3BucketReadAccess enum to represent open or secure read access
- Include `s3BucketsConfig` block sample
- Update Deployer to merge S3 config with each verticle's config
…changes

- Change both to load S3 config into S3ConfigsHolder object
- Note that for ProcessesRunnerImpl, more changes are required to make it work
for processes properly
S3ConfigsHolder
---------------
- Added `default` bucket identifier key for existing data to work fine

ProcessVerticle
---------------
- Created `HttpClient` instance and set to shared so that hopefully all processes can use the shared version

ProcessRunnerImpl
-----------------
- Changed `getS3Object()` -> `getS3Config()` to get `S3Config` object based on process input
	- For now, just getting `default`

Processes
---------
- Updated all processes to take `S3Config` as parameter instead of `DataFromS3`
	- Instead creating `DataFromS3` object in process constructor using `S3Config` object and shared `HttpClient`
- Updated `TilesOnboardingFromExistingFeatureProcess` to use `S3Config` object
- In Deployer, if `S3BucketsConfig` JSON object is not present/empty in config, don't start server
- Force uppercase for `readAccess` key in config
- Fix boolean check for `pathBasedAccess` key
…ied APIs that use S3 to check for bucket ID from that column

- Added migration for adding the column
- Updated download coverage and download asset APIs to get S3 config based on `s3_bucket_id` from DB
- Updated tiles API to check DB for the bucket ID - it previously was not doing any DB calls for tile stuff
- Remove the part of the migration that removes the DEFAULT constraint - this is affecting
other PRs
- Will add that migration later
- Add migration to update all existing processes in `processes_table` to have the mentioned input key
- Update `ProcessRunnerImpl`
	- Change `validateInput` logic to send back 400 Bad Request error in case of missing/empty keys along w/ offending keys
	- Checking for the `s3BucketIdentifier` key from input now
- Update process tests
	- Update tests for onboarding, appending and tiles metadata onboarding tests to add the extra input key `s3BucketIdentifier`
	with value `default`
- Running migration V19.2 broke other tests, so instead of relying on migration for tests,
using the maven-sql-plugin scripts to update process inputs before test and reset them after
tests
- Fix s3BucketIdentifier constant in tests, was using wrong one
…ge and getTile methods

- Was previously just throwing an OgcException, but the exception cannot be handled. So instead,
convert to compose and return a failed future
- Had to remove the extra onFailure blocks since the status code was getting overwritten. Now
onFailure just fails the RoutingContext and it's handled in the FailureHandler
…ess input update

- Updated the process to use S3Config fields also
- Removed 19.2 migration because it's been added in the maven-sql-plugin - will make it a migration later on
- Added maven-sql-plugin scripts to remove `bucketName` and `region` from the process inputs before tests
and then add them back afterwards
- Fixed small bug in process introduced in previous commit
- Added teardown SQL script for deleting tiles data from DB
…aven-sql-plugin for now

- Conflicting migration numbers were causing issues
- Will later make this a migration
- Modified processes to use S3Config wherever applicable
- Added temp maven-sql-plugin scripts to add `s3BucketIdentifier` key in input and remove `bucketName`
- Updated tests
@ThorodanBrom ThorodanBrom force-pushed the multiple-buckets-init branch from 04f97bf to 334aed5 Compare March 4, 2025 13:19
@ThorodanBrom
Copy link
Collaborator Author

retest this please

… IDs and read access

- Added API to OGC router, modified OpenAPI spec
- Changed read access enum from OPEN, SECURE to PUBLIC, PRIVATE resp.
- Updated S3ConfigsHolder class to organize the bucket ID, read access info as
a JSON array on initialization
- Some migrations were previously added in older commits but were removed
due to conflicts
- V20.1 - Set default for `id` field and unique constraint for `title` field in `processes_table`
- V20.2 - Add processes into `processes_table` if they don't exist already
- V21.1 - Add `s3_bucket_id` column to all tables that work w/ S3
- V21.2 - Add `s3BucketIdentifier` input for all processes that work w/ S3
- V21.3 - Remove unnecessary inputs from processes that work w/ S3, `S3BucketIdentifier` removes the need for them
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 this pull request may close these issues.

1 participant