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.

Sign up for GitHub

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

Jump to bottom

feat: Added support to specify AWS profile through S3 instructions #6130

Merged

malhotrashivam merged 18 commits into deephaven:main from malhotrashivam:sm-s3-profile

Oct 3, 2024

Contributor

malhotrashivam commented Sep 25, 2024 •

edited

Loading

Closes #6121

Documentation update: link


          Initial commit

81179cb

malhotrashivam added feature request DocumentationNeeded ReleaseNotesNeeded s3 iceberg labels

malhotrashivam added this to the 0.37.0 milestone

malhotrashivam requested a review from devinrsmith

September 25, 2024 17:13

malhotrashivam self-assigned this

malhotrashivam requested review from chipkent, jmao-denver and rcaudy as code owners

September 25, 2024 17:13

malhotrashivam changed the title ~~Added support to specify AWS profile through S3 instructions~~ feat: Added support to specify AWS profile through S3 instructions

devinrsmith reviewed

View reviewed changes

extensions/parquet/table/src/test/java/io/deephaven/parquet/table/S3ParquetLocalStackTest.java Outdated Show resolved Hide resolved

extensions/s3/src/test/java/io/deephaven/extensions/s3/testlib/SingletonContainers.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/ProfileCredentials.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java Outdated Show resolved Hide resolved


          Review with Devin

e87d3fc

malhotrashivam requested a review from devinrsmith

September 25, 2024 20:09

malhotrashivam commented

View reviewed changes

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3CompletableOutputStream.java Show resolved Hide resolved

devinrsmith reviewed

View reviewed changes

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Utils.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Utils.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Utils.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java Outdated Show resolved Hide resolved

malhotrashivam added 3 commits

September 25, 2024 16:39


          Review with Devin Part 2

d6930cf


          Minor compilation fix

de0f602


          Review with Devin part 3

cfe7106

malhotrashivam requested a review from devinrsmith

September 25, 2024 22:44

malhotrashivam commented

View reviewed changes

extensions/s3/src/main/java/io/deephaven/extensions/s3/DefaultCredentials.java Show resolved Hide resolved

devinrsmith previously approved these changes

View reviewed changes


          Added a profile credentials

9f04170

malhotrashivam dismissed devinrsmith’s stale review via

9f04170

September 26, 2024 00:51

malhotrashivam requested a review from devinrsmith

September 26, 2024 00:52

devinrsmith reviewed

View reviewed changes

extensions/s3/src/main/java/io/deephaven/extensions/s3/ProfileCredentials.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java Outdated Show resolved Hide resolved


          Review with Devin contd.

356b81d

devinrsmith reviewed

View reviewed changes

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/ProfileCredentials.java Outdated Show resolved Hide resolved

extensions/s3/src/main/java/io/deephaven/extensions/s3/ProfileCredentials.java Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

malhotrashivam dismissed devinrsmith’s stale review via

a0a817d

September 27, 2024 21:22

rcaudy reviewed

View reviewed changes

Member

rcaudy left a comment

Even better. Merge when Python is approved.

devinrsmith previously approved these changes

View reviewed changes

jmao-denver reviewed

View reviewed changes

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

jmao-denver reviewed

View reviewed changes

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

chipkent reviewed

View reviewed changes

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved


          Resolving python comments

9ab34c6

malhotrashivam dismissed devinrsmith’s stale review via

9ab34c6

September 30, 2024 16:13

chipkent reviewed

View reviewed changes

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Show resolved Hide resolved


          Improving python error message

7bbbc1e

chipkent reviewed

View reviewed changes

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

malhotrashivam added 3 commits

September 30, 2024 14:57


          Minor python code reordering

30ec2c3


          More tweaks to python comments

b748f03


          Moved S3Credentials to a different python module

a734ebe

jmao-denver reviewed

View reviewed changes

Contributor

jmao-denver left a comment

Should S3Credentials be in its own module or just folded into s2.py? I favor the latter for simplicity reason.


          Python review continued

b764663

Contributor Author

malhotrashivam commented Oct 2, 2024

Should S3Credentials be in its own module or just folded into s2.py? I favor the latter for simplicity reason.

Done

chipkent reviewed

View reviewed changes

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3_credentials.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3_credentials.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

malhotrashivam mentioned this pull request

Remove deprecated fields created in #6130 #6165

Closed


          More python review comments resolved

a20c844

chipkent reviewed

View reviewed changes

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/experimental/s3.py Show resolved Hide resolved


          Resolving more python comments

10906c9

chipkent approved these changes

View reviewed changes

devinrsmith approved these changes

View reviewed changes

rcaudy approved these changes

View reviewed changes

jmao-denver self-requested a review

October 3, 2024 17:26

jmao-denver approved these changes

View reviewed changes

Contributor

jmao-denver left a comment

LGTM

malhotrashivam merged commit 15ff4f5 into deephaven:main

17 checks passed

github-actions bot locked and limited conversation to collaborators

Contributor

deephaven-internal commented Oct 3, 2024

Labels indicate documentation is required. Issues for documentation have been opened:

Community: deephaven/deephaven-docs-community#319

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

jjbrosnan jjbrosnan left review comments

devinrsmith devinrsmith approved these changes

rcaudy rcaudy approved these changes

chipkent chipkent approved these changes

jmao-denver jmao-denver approved these changes

Labels

DocumentationNeeded feature request iceberg ReleaseNotesNeeded s3