12036 remove minio as s3 storage option#91
Conversation
…tions via API (no superuser)
0e9f779 to
420101b
Compare
There was a problem hiding this comment.
Pull request overview
Removes MinIO as a supported/dev S3-compatible storage option and standardizes S3 testing/dev setup around LocalStack, including an explicit “no redirect” LocalStack driver to exercise Dataverse’s proxy download path.
Changes:
- Updated S3 integration tests to use
localstack_noredirect(no download redirect) instead of MinIO for non-direct upload/download coverage. - Removed MinIO from dev docker-compose configurations and associated dev-start script setup.
- Updated installation docs to remove MinIO-specific guidance and references.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/edu/harvard/iq/dataverse/api/S3AccessIT.java | Switches non-direct upload test coverage from MinIO to LocalStack no-redirect driver; adjusts bucket usage and download behavior. |
| src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java | Removes MinIO-specific mention from object-tagging warning comment. |
| scripts/dev/dev-start-frd.sh | Removes creation of MinIO dev volume directory. |
| docker-compose-dev.yml | Removes MinIO service/config; adds localstack_noredirect S3 driver config. |
| conf/keycloak/docker-compose-dev.yml | Same as above for Keycloak dev stack. |
| doc/sphinx-guides/source/installation/config.rst | Removes MinIO mentions; updates S3 endpoint example. |
| doc/sphinx-guides/source/installation/big-data-support.rst | Removes MinIO CORS tooling references and MinIO tagging note. |
Comments suppressed due to low confidence (1)
doc/sphinx-guides/source/installation/big-data-support.rst:112
- After removing the Minio-specific docs, this section still instructs users to run
mc cors set ...(Minio client) and includescors.xmlunder the AWS CLI tab. Either restore a separate Minio/"mc" tab, replace this with an AWS CLI equivalent, or remove themcinstructions to keep the guidance consistent.
:language: xml
Proceed with making the changes:
:code:`mc cors set <STORE_NAME>/<BUCKET_NAME> ./cors.xml`
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static final String BUCKET_NAME = "mybucket"; | ||
| static final String BUCKET_NAME_NOREDIRECT = "mybucket-noredirect"; | ||
| static S3Client s3localstack = null; |
| -Ddataverse.files.localstack_noredirect.label=LocalStackNoRedirect | ||
| -Ddataverse.files.localstack_noredirect.custom-endpoint-url=http://localstack:4566 | ||
| -Ddataverse.files.localstack_noredirect.custom-endpoint-region=us-east-2 | ||
| -Ddataverse.files.localstack_noredirect.bucket-name=mybucket-noredirect |
| -Ddataverse.files.localstack_noredirect.label=LocalStackNoRedirect | ||
| -Ddataverse.files.localstack_noredirect.custom-endpoint-url=http://localstack:4566 | ||
| -Ddataverse.files.localstack_noredirect.custom-endpoint-region=us-east-2 | ||
| -Ddataverse.files.localstack_noredirect.bucket-name=mybucket-noredirect |
Coverage Report for CI Build 114Warning No base build found for commit Coverage: 24.957%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
|
@copilot Please investigate this test. It passes just fine in the Jenkins run, fails only in containers: 1 expectation failed. { } |
I investigated the container workflow failure (run 26474971605) and applied a fix. The issue was that |
|
Yepp, AI is almost certainly going to take my job |
ca0b4cf to
940bd09
Compare
7e5b3e3 to
84b0d18
Compare
Add source-level OpenAPI annotations across Dataverse REST resources, including class tags, operation summaries/descriptions, parameter and request-body docs, API key security metadata, and response schema fixes for generated OpenAPI/MCP tooling. IQSS#12437 describes this effort in more details
…vel-description Added support for HTML in JSF UI for file description
…payara_files Replace deprecated glassfish files with payara-* files in WEB-INF
…k/docker-compose-dev.yml
…load Switch testNonDirectUpload from localstack1 (upload-redirect=true, download-redirect=true) to the new localstack_noredirect driver (both redirects disabled), so the test genuinely exercises the non-redirect proxy-through-Dataverse code path. Also replace the plain downloadFile call with downloadFileNoRedirect and assert statusCode(200). This makes the assertion self-documenting: a 303 response would now cause an explicit test failure instead of being silently followed by RestAssured.
…alstack_noredirect Using the same bucket name as localstack1 would cause a collision in the test environment when tasks/localstack_create_bucket.yml runs aws s3 mb on each bucket entry. Use mybucket-noredirect to avoid this. Update driver configs in both docker-compose files and switch S3AccessIT.testNonDirectUpload to use the new BUCKET_NAME_NOREDIRECT constant.
Updated comment for clarity regarding S3 tags implementation.
84b0d18 to
b705ef1
Compare
What this PR does / why we need it:
Which issue(s) this PR closes:
Special notes for your reviewer:
Suggestions on how to test this:
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation: