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! |
2 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 deprecates the oldgcs.service.pathproperty and prefers thegcs.service.host. When we parse the properties we're falling back to the old one incase consumers were relying on this but I've marked the property as deprecated with the aim of removing it at some point in the future.Java
py-iceberg
What changes are included in this PR?
Are these changes tested?
AI Disclosure
Fixed up #[allow(deprecated)] for me.