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

Support s3.signer.endpoint for nessie #1029

Merged
merged 9 commits into from
Aug 12, 2024
Merged

Conversation

guitcastro
Copy link
Contributor

This PR honor s3.signer.endpoint property, fixing the first point in #1028.

pyiceberg/io/fsspec.py Outdated Show resolved Hide resolved
pyiceberg/io/fsspec.py Outdated Show resolved Hide resolved
pyiceberg/io/fsspec.py Outdated Show resolved Hide resolved
@guitcastro
Copy link
Contributor Author

@Fokko @ndrluis I've applied the code suggestions. Can you please review again?

mkdocs/docs/configuration.md Outdated Show resolved Hide resolved
Co-authored-by: Fokko Driesprong <[email protected]>
This was referenced Aug 9, 2024
@ndrluis ndrluis added this to the PyIceberg 0.8.0 release milestone Aug 9, 2024
Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

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

LGTM, added a few minor comments

mkdocs/docs/configuration.md Outdated Show resolved Hide resolved
pyiceberg/io/__init__.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

@guitcastro Can you run make lint to fix the style issues:

File : error: indent/outdent mismatch: 7.
Warning: Failed formatting content of a yaml code block (line 278 before formatting). Filename: /home/runner/work/iceberg-python/iceberg-python/mkdocs/docs/configuration.md
Warning: Failed formatting content of a python code block (line 32 before formatting). Filename: /home/runner/work/iceberg-python/iceberg-python/mkdocs/docs/how-to-release.md
Warning: Failed formatting content of a python code block (line 1046 before formatting). Filename: /home/runner/work/iceberg-python/iceberg-python/mkdocs/docs/api.md
Warning: Failed formatting content of a python code block (line 1108 before formatting). Filename: /home/runner/work/iceberg-python/iceberg-python/mkdocs/docs/api.md

I think this is good to get in 👍

@guitcastro
Copy link
Contributor Author

@Fokko Done!

@Fokko Fokko merged commit 2e73a41 into apache:main Aug 12, 2024
7 checks passed
@Fokko
Copy link
Contributor

Fokko commented Aug 12, 2024

Thanks @guitcastro for picking this up, and thanks @ndrluis and @kevinjqliu for the review 🙌

sungwy pushed a commit to sungwy/iceberg-python that referenced this pull request Dec 7, 2024
* s3_signer_endpoint

* prune any trailing whitespaces

Co-authored-by: Fokko Driesprong <[email protected]>

* fallback to default value instead of "endpoint" property

Co-authored-by: Fokko Driesprong <[email protected]>

* fix test_s3v4_rest_signer_endpoint

* Fix missing backtick

Co-authored-by: Fokko Driesprong <[email protected]>

* rename S3_SIGNER_ENDPOINT_DEFAULT_VALUE to S3_SIGNER_ENDPOINT_DEFAULT

* fix s3.signer.endpoint docs

* fk typo in signer

* fix fmt

---------

Co-authored-by: guilhermecastro <[email protected]>
Co-authored-by: Fokko Driesprong <[email protected]>
sungwy pushed a commit to sungwy/iceberg-python that referenced this pull request Dec 7, 2024
* s3_signer_endpoint

* prune any trailing whitespaces

Co-authored-by: Fokko Driesprong <[email protected]>

* fallback to default value instead of "endpoint" property

Co-authored-by: Fokko Driesprong <[email protected]>

* fix test_s3v4_rest_signer_endpoint

* Fix missing backtick

Co-authored-by: Fokko Driesprong <[email protected]>

* rename S3_SIGNER_ENDPOINT_DEFAULT_VALUE to S3_SIGNER_ENDPOINT_DEFAULT

* fix s3.signer.endpoint docs

* fk typo in signer

* fix fmt

---------

Co-authored-by: guilhermecastro <[email protected]>
Co-authored-by: Fokko Driesprong <[email protected]>
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.

4 participants