[WIP] Add possibility to parameterize S3 service URL #137
Conversation
Codecov Report
@@ Coverage Diff @@
## master #137 +/- ##
======================================
Coverage 93.5% 93.5%
======================================
Files 21 21
Lines 1119 1119
======================================
Hits 1047 1047
Misses 72 72
|
|
Happy to take this change! A few questions:
Update: Heroku one-button deploy worked like a charm for a test server.
|
There was a problem hiding this comment.
Looking great @YevheniiSemendiak, thanks! Just a couple small things to tweak.
Once those are done, I'll probably move this over to a PR branch on the repo so that I can get the integration tests that use a live backend properly configured before merging to main.
| cp1 = client.CloudPath("s3://cloudpathlib-test-bucket/") | ||
| # or pass the client as keyword argument | ||
| cp2 = CloudPath("s3://cloudpathlib-test-bucket/", client=client) | ||
| ``` |
There was a problem hiding this comment.
I might set this up to show three scenarios:
from cloudpathlib import S3Client, CloudPath
# create a client pointing to the endpoint
client = S3Client(endpoint_url="http://my.s3.server:1234")
# option 1: use the client to create paths
cp1 = client.CloudPath("s3://cloudpathlib-test-bucket/")
# option 2: pass the client as keyword argument
cp2 = CloudPath("s3://cloudpathlib-test-bucket/", client=client)
# option3: set this client as the default so it is used in any future paths
client.set_as_default_client()
cp3 = CloudPath("s3://cloudpathlib-test-bucket/")There was a problem hiding this comment.
done here fc113dc
| @@ -212,6 +219,49 @@ def s3_rig(request, monkeypatch, assets_dir): | |||
| bucket.objects.filter(Prefix=test_dir).delete() | |||
|
|
|||
|
|
|||
| @fixture() | |||
| def custom_s3_rig(request, monkeypatch, assets_dir): | |||
There was a problem hiding this comment.
Let's add a docstring that this is used for testing things like MinIO/ceph
There was a problem hiding this comment.
done here 762c884
tests/conftest.py
Outdated
| custom_secret_key = os.getenv("CUSTOM_S3_SECRET_KEY") | ||
| test_dir = create_test_dir_name(request) | ||
|
|
||
| # Upload test assets |
There was a problem hiding this comment.
Let's keep the same if os.getenv("USE_LIVE_CLOUD") == "1": logic as in the S3 one so that local tests are fully mocked (endpoint_url is basically ignored in the mock), but we are running the integration tests against live backends we do this copying.
There was a problem hiding this comment.
done here 762c884
thanks for the clarification 🙂
* [WIP] Add possibility to parameterize S3 service URL (#137) * Add possibility to parameterize S3 service URL * add usage description into the docs * add rig for custom S3 servers * address PR review: add cods to rig, allow mock * forgot to commit tests/conftest.py :) * Run MinIO test rig * Update docs/docs/authentication.md * Simplify custom S3 auth; fix boto3 session state leak between fixtures * Set env vars on live test workflow * Remove comment about MinIO. Test passed so it seems fine * Fix incorrect secrets reference * Skip default client reset for custom s3 endpoint * Skip mod time touch test for custom s3 * Remove unneeded import * Update changelog Co-authored-by: Yevhenii Semendiak <semendyak@gmail.com> Co-authored-by: Jay Qi <2721979+jayqi@users.noreply.github.com> Co-authored-by: Jay Qi <jayqi@users.noreply.github.com>
Just a minor thing, related to #136
I didn't found contribution guides, so this is potentially draft PR.
Nevertheless, WDYT about the possibility to support non-AWS S3 deployments?
Re the PR: what else should be added?
P.S. Thank you folks for an awesome lib 👍