-
Notifications
You must be signed in to change notification settings - Fork 433
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
feat: support reading tables via Unity Catalog provided credentials #3078
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3078 +/- ##
==========================================
- Coverage 72.08% 71.88% -0.21%
==========================================
Files 134 134
Lines 43360 43480 +120
Branches 43360 43480 +120
==========================================
- Hits 31258 31255 -3
- Misses 10085 10203 +118
- Partials 2017 2022 +5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor nits to maybe look at?
One question. Do i read that right, that we are only supporting AWS for now?
cd79543
to
e95b915
Compare
the build failures should hopefully be addressed by #3138 - could you merge / rebase? |
commit cd79543 Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Thu Jan 16 16:05:10 2025 -0500 feat: support reading tables via Unity Catalog provided credentials Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com> commit 5a9ab33 Merge: 9551413 d6e1fbf Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Thu Jan 16 15:48:52 2025 -0500 Merge remote-tracking branch 'mine/unity-catalog-read' into unity-catalog-read # Conflicts: # crates/catalog-unity/Cargo.toml commit 9551413 Merge: ed8ac96 7f8b2ab Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Thu Jan 16 15:47:41 2025 -0500 feat: support reading tables via Unity Catalog provided credentials Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com> commit d6e1fbf Merge: ed8ac96 7f8b2ab Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Thu Jan 16 15:47:41 2025 -0500 Merge remote-tracking branch 'mine/unity-catalog-read' into unity-catalog-read # Conflicts: # crates/catalog-unity/Cargo.toml commit ed8ac96 Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Sun Dec 22 12:34:42 2024 -0500 feat: support reading tables via Unity Catalog provided credentials Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com> commit bb81a51 Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Sun Dec 22 12:34:42 2024 -0500 feat: support reading tables via Unity Catalog provided credentials Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com> commit 17a9d1b Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Sun Dec 22 12:34:42 2024 -0500 feat: support reading tables via Unity Catalog provided credentials Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com> commit 7f8b2ab Merge: 9a6e48e 24bbcdb Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Thu Dec 26 11:43:29 2024 -0500 Merge branch 'main' into unity-catalog-read commit 9a6e48e Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Sun Dec 22 12:34:42 2024 -0500 feat: support reading tables via Unity Catalog provided credentials Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com> commit 1c3fe85 Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Sun Dec 22 12:34:42 2024 -0500 feat: support reading tables via Unity Catalog provided credentials Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com> commit f7cc78b Author: Stephen Carman <hntd187@users.noreply.github.com> Date: Sun Dec 22 12:34:42 2024 -0500 feat: support reading tables via Unity Catalog provided credentials Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com> Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com>
Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com>
0617459
to
a2c910c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some small nits. Also I suggest we add an integration test to the CI, this would also help with setting up python tests later on
Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com>
crates/catalog-unity/src/lib.rs
Outdated
.unwrap(); | ||
assert!(matches!(get_table_response, GetTableResponse::Success(_))); | ||
.await; | ||
dbg!(&get_table_response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a dbg! in there : )
Signed-off-by: Stephen Carman <hntd187@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! :D
Description
You can read tables via UC provided creds.
Related Issue(s)
Documentation
Temp Creds API