SteNicholas commented on code in PR #274:
URL: https://github.com/apache/paimon-cpp/pull/274#discussion_r4036417356
##########
src/paimon/rest/rest_catalog.cpp:
##########
@@ -438,6 +453,23 @@ std::shared_ptr<FileSystem> RestCatalog::GetFileSystem()
const {
return fs_;
}
+Result<std::shared_ptr<FileSystem>> RestCatalog::GetTableFileSystem(
+ const Identifier& identifier) const {
+ if (!data_token_enabled_) {
+ return fs_;
+ }
Review Comment:
**Preserve an explicitly supplied catalog filesystem**. When
`Catalog::Create(..., custom_fs)` is combined with `data-token.enabled=true`
(including when enabled by `/v1/config`), this method ignores `fs_` and always
returns a new `RestTokenFileSystem`. The read/write/commit paths using
`WithCatalog` therefore discard the caller's filesystem and its
`CredentialProvider`: `BuildFileSystem()` reconstructs a built-in delegate from
options with `specified_file_system=nullptr`. This breaks the newly documented
`Catalog::Create` injection path, which promises to use the supplied filesystem
as-is, and can send custom-scheme accesses to the default local backend. Please
retain whether a filesystem was explicitly supplied and preserve that override
here, with a test that enables data tokens while injecting a custom filesystem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
