fix(gcs): Fix GCS host property - #2965
Conversation
01f5eed to
11e476b
Compare
| #[deprecated(since = "0.10.1", note = "use GCS_SERVICE_HOST instead")] | ||
| pub const GCS_SERVICE_PATH: &str = "gcs.service.path"; |
There was a problem hiding this comment.
If we prefer to hard break here and include this in the release docs for the next release, I'm also happy to do that. Wasn't sure what the right thing to do here was.
|
cc: @Xuanwo looks like in the original PR you called this our #520 (comment) but it wasn't updated https://github.com/apache/iceberg-rust/pull/520/changes#diff-455ed5286778e4be48bfafeadaad6ae688ac69bdcfc17c34fec15cdbc55b5e25R32. Do you have thoughts on this PR? |
anoopj
left a comment
anoopj
left a comment
There was a problem hiding this comment.
The code change looks good to me, but the PR description says "This PR depreciates the old gcs.service.host property and prefers the gcs.service.path"
I think you meant the opposite. (also typo: should be "deprecates")
| /// endpoints vended by a REST catalog or copied from another engine were | ||
| /// silently ignored. It is still read as a fallback for backwards | ||
| /// compatibility. | ||
| #[deprecated(since = "0.10.1", note = "use GCS_SERVICE_HOST instead")] |
There was a problem hiding this comment.
May need to bump this since this is already out?
There was a problem hiding this comment.
Ah yes, I guess 0.11.0 is more correct?
|
Good catch, thank you! |
| /// endpoints vended by a REST catalog or copied from another engine were | ||
| /// silently ignored. It is still read as a fallback for backwards | ||
| /// compatibility. | ||
| #[deprecated(since = "0.11.0", note = "use GCS_SERVICE_HOST instead")] |
There was a problem hiding this comment.
I prefer to remove it directly.
There was a problem hiding this comment.
Have removed altogether
7775da5 to
403f5f2
Compare
Rename the GCS endpoint property from `gcs.service.path` to `gcs.service.host` to match the Iceberg Java and PyIceberg implementations. The old key never existed in those implementations, so endpoints vended by a REST catalog or copied from another engine were silently ignored.
403f5f2 to
0e8050b
Compare
3 participants
Which issue does this PR close?
Java and py-icebery both use
gcs.service.hostas the path to the bucket whereas we're currently usinggcs.service.pathwhich makes interop a little difficult between engines and catalogs. This PR renames the property fromgcs.service.pathtogcs.service.hostto match.Java
py-iceberg
What changes are included in this PR?
Are these changes tested?
AI Disclosure