Repository navigation
Conversation
| bucket = "test-create-bucket" | ||
|
|
||
| try: | ||
| result = runner.invoke(create_bucket, [bucket]) |
There was a problem hiding this comment.
Could we invoke s3 create-bucket through the application CLI?
Calling create_bucket directly bypasses the command group and entry-point registration added here.
There was a problem hiding this comment.
Because invenio-s3 doesn't include the full invenio stack of dependencies, it can't run invenio commands directly (missing psycopg2 from invenio-db). I don't think messing with the dependency stack is worth the cleaner tests.
c86feee to
f6ae3d2
Compare
| fs = s3fs.S3FileSystem(**info) | ||
|
|
||
| try: | ||
| fs.mkdir(bucket) |
There was a problem hiding this comment.
Should we handle S3_REGION_NAME="us-east-1" explicitly here?
AWS requires CreateBucketConfiguration.LocationConstraint to be omitted for us-east-1 we mention that in the docs as well, but s3fs.mkdir() sends it when the region is configured explicitly.
see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateBucket.html
There was a problem hiding this comment.
I see where this is coming from....but I don't think it's an actual issue since other people haven't reported it for s3fs. Changing it makes the code a lot more complicated. I think we can wait until we get an actual user report.
| fs.mkdir(bucket) | ||
| except FileExistsError: | ||
| click.secho(f"Bucket {bucket} already exists.", fg="yellow") | ||
| return |
There was a problem hiding this comment.
Should we continue to the CORS step when the bucket already exists?
If put_bucket_cors fails after creation, rerunning the command currently returns on FileExistsError, so CORS can never be retried.
2 participants
❤️ Thank you for your contribution!
Description
Minio used to create a default bucket, but RustFS doesn't do that. In the cli bucket creation was added in the services inveniosoftware/docker-services-cli#56, but for a cookiecutter RDM instance it's kind of awkward to do it from either docker or invenio-cli. invenio-s3 is already installed and knows all about the s3 configuration.
I don't anticipate us needing to expand the cli, since most people will manage S3 externally.
Claude assisted with this code.
Checklist
Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:
Frontend
Reminder
By using GitHub, you have already agreed to the GitHub’s Terms of Service including that: