You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "fresh-borzoni (via GitHub)" <gi...@apache.org> on 2026/04/04 09:09:36 UTC
[PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
fresh-borzoni opened a new pull request, #2997:
URL: https://github.com/apache/fluss/pull/2997
## Summary
The `client.fs.*` prefix convention (introduced in #938) allows Flink clients to pass filesystem-specific config to Fluss. However, the config was silently ignored by all filesystem plugins due to two bugs.
This PR fixes both issues and adds short prefix support (`oss.`, `obs.`, `azure.`) to the remaining plugins, making the config convention work consistently across all filesystems.
Fixes #2996
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "morazow (via GitHub)" <gi...@apache.org>.
morazow commented on code in PR #2997:
URL: https://github.com/apache/fluss/pull/2997#discussion_r3086956034
##########
fluss-filesystems/fluss-fs-oss/src/main/java/org/apache/fluss/fs/oss/OSSFileSystemPlugin.java:
##########
@@ -54,7 +54,9 @@ public class OSSFileSystemPlugin implements FileSystemPlugin {
* In order to simplify, we make fluss oss configuration keys same with hadoop oss module. So,
* we add all configuration key with prefix `fs.oss` in fluss conf to hadoop conf
Review Comment:
Here also, Javadoc update required.
##########
fluss-filesystems/fluss-fs-obs/src/main/java/org/apache/fluss/fs/obs/OBSFileSystemPlugin.java:
##########
@@ -41,7 +41,9 @@ public class OBSFileSystemPlugin implements FileSystemPlugin {
* In order to simplify, we make fluss obs configuration keys same with hadoop obs module. So,
* we add all configuration key with prefix `fs.obs` in fluss conf to hadoop conf
Review Comment:
Javadoc comment needs to be updated 🤝
Maybe remove the `fs.obs` and put generic description.
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/AzureFileSystemPlugin.java:
##########
Review Comment:
Is this a bug from before?
With the normalization changes in `getHadoopConfiguration` method, should this be `hadoopConfig` instead of `flussConfig`?
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/AzureFileSystemPlugin.java:
##########
@@ -43,7 +43,7 @@
* (abfs, abfss, wasb, wasbs) based on Azure HDFS support in the hadoop-azure module.
*/
abstract class AzureFileSystemPlugin implements FileSystemPlugin {
- private static final String[] FLUSS_CONFIG_PREFIXES = {"fs.azure."};
+ private static final String[] FLUSS_CONFIG_PREFIXES = {"azure.", "fs.azure."};
Review Comment:
I'd suggest to extend the corresponding [AzureFileSystemPluginTest.java#L42](https://github.com/fresh-borzoni/fluss/blob/311304d48871f8709daef81d141eca8f609378f3/fluss-filesystems/fluss-fs-azure/src/test/java/org/apache/fluss/fs/azure/AzureFileSystemPluginTest.java#L42) test with new key.
Same for OBS and OSS changes 👍
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on PR #2997:
URL: https://github.com/apache/fluss/pull/2997#issuecomment-4251107880
@affo @morazow Can you pls take a look?
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "morazow (via GitHub)" <gi...@apache.org>.
morazow commented on code in PR #2997:
URL: https://github.com/apache/fluss/pull/2997#discussion_r3093371806
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/token/AzureDelegationTokenProvider.java:
##########
@@ -49,15 +46,19 @@ public class AzureDelegationTokenProvider {
public AzureDelegationTokenProvider(String scheme, Configuration conf) {
this.scheme = scheme;
- this.clientId = conf.get(CLIENT_ID);
- this.clientSecret = conf.get(CLIENT_SECRET);
- this.authEndpoint = conf.get(ENDPOINT_KEY);
+ String clientIdKey = AzureFileSystemOptions.CLIENT_ID.key();
+ String clientSecretKey = AzureFileSystemOptions.CLIENT_SECRET.key();
+ String endpointKey = AzureFileSystemOptions.ENDPOINT_KEY.key();
+
+ this.clientId = conf.get(clientIdKey);
+ this.clientSecret = conf.get(clientSecretKey);
+ this.authEndpoint = conf.get(endpointKey);
Review Comment:
Ahh yeah, right. Yes, no looks fine from my side 🤝
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on code in PR #2997:
URL: https://github.com/apache/fluss/pull/2997#discussion_r3087613571
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/AzureFileSystemPlugin.java:
##########
Review Comment:
Good catch, ty, fixed this pre-existing bug as well 👍
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on PR #2997:
URL: https://github.com/apache/fluss/pull/2997#issuecomment-4253420990
@morazow Ty for the review, addressed, PTAL 🙏
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "morazow (via GitHub)" <gi...@apache.org>.
morazow commented on code in PR #2997:
URL: https://github.com/apache/fluss/pull/2997#discussion_r3091963276
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/token/AzureDelegationTokenProvider.java:
##########
@@ -49,15 +46,19 @@ public class AzureDelegationTokenProvider {
public AzureDelegationTokenProvider(String scheme, Configuration conf) {
this.scheme = scheme;
- this.clientId = conf.get(CLIENT_ID);
- this.clientSecret = conf.get(CLIENT_SECRET);
- this.authEndpoint = conf.get(ENDPOINT_KEY);
+ String clientIdKey = AzureFileSystemOptions.CLIENT_ID.key();
+ String clientSecretKey = AzureFileSystemOptions.CLIENT_SECRET.key();
+ String endpointKey = AzureFileSystemOptions.ENDPOINT_KEY.key();
+
+ this.clientId = conf.get(clientIdKey);
+ this.clientSecret = conf.get(clientSecretKey);
+ this.authEndpoint = conf.get(endpointKey);
Review Comment:
Let's revert this change, maybe it was unintentional. Anyway, we can inline the keys on get(...) call or revert to previous chage, not to confuse with the PR changes.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "morazow (via GitHub)" <gi...@apache.org>.
morazow commented on code in PR #2997:
URL: https://github.com/apache/fluss/pull/2997#discussion_r3091963276
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/token/AzureDelegationTokenProvider.java:
##########
@@ -49,15 +46,19 @@ public class AzureDelegationTokenProvider {
public AzureDelegationTokenProvider(String scheme, Configuration conf) {
this.scheme = scheme;
- this.clientId = conf.get(CLIENT_ID);
- this.clientSecret = conf.get(CLIENT_SECRET);
- this.authEndpoint = conf.get(ENDPOINT_KEY);
+ String clientIdKey = AzureFileSystemOptions.CLIENT_ID.key();
+ String clientSecretKey = AzureFileSystemOptions.CLIENT_SECRET.key();
+ String endpointKey = AzureFileSystemOptions.ENDPOINT_KEY.key();
+
+ this.clientId = conf.get(clientIdKey);
+ this.clientSecret = conf.get(clientSecretKey);
+ this.authEndpoint = conf.get(endpointKey);
Review Comment:
Let's revert this change, maybe it was unintentional. Anyway, we can inline the keys on get(...) call or revert to previous chage, not to confuse with the PR changes.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "morazow (via GitHub)" <gi...@apache.org>.
morazow commented on code in PR #2997:
URL: https://github.com/apache/fluss/pull/2997#discussion_r3093374921
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/token/AzureDelegationTokenProvider.java:
##########
@@ -49,15 +46,19 @@ public class AzureDelegationTokenProvider {
public AzureDelegationTokenProvider(String scheme, Configuration conf) {
this.scheme = scheme;
- this.clientId = conf.get(CLIENT_ID);
- this.clientSecret = conf.get(CLIENT_SECRET);
- this.authEndpoint = conf.get(ENDPOINT_KEY);
+ String clientIdKey = AzureFileSystemOptions.CLIENT_ID.key();
+ String clientSecretKey = AzureFileSystemOptions.CLIENT_SECRET.key();
+ String endpointKey = AzureFileSystemOptions.ENDPOINT_KEY.key();
+
+ this.clientId = conf.get(clientIdKey);
+ this.clientSecret = conf.get(clientSecretKey);
+ this.authEndpoint = conf.get(endpointKey);
Review Comment:
I already approved it 👍
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on code in PR #2997:
URL: https://github.com/apache/fluss/pull/2997#discussion_r3087618497
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/AzureFileSystemPlugin.java:
##########
@@ -43,7 +43,7 @@
* (abfs, abfss, wasb, wasbs) based on Azure HDFS support in the hadoop-azure module.
*/
abstract class AzureFileSystemPlugin implements FileSystemPlugin {
- private static final String[] FLUSS_CONFIG_PREFIXES = {"fs.azure."};
+ private static final String[] FLUSS_CONFIG_PREFIXES = {"azure.", "fs.azure."};
Review Comment:
I added this test, but there are no OBS/OSS plugin dedicated tests, so probably out of scope here.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "polyzos (via GitHub)" <gi...@apache.org>.
polyzos merged PR #2997:
URL: https://github.com/apache/fluss/pull/2997
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "affo (via GitHub)" <gi...@apache.org>.
affo commented on PR #2997:
URL: https://github.com/apache/fluss/pull/2997#issuecomment-4260419968
Btw, there is also something similar happening for lake tiering for which you have to specify twice a prefix, e.g.: `datalake.iceberg.iceberg....`, I will try to find a bit of space to address that 😓
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on PR #2997:
URL: https://github.com/apache/fluss/pull/2997#issuecomment-4186811819
@luoyuxia @polyzos PTAL 🙏
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "polyzos (via GitHub)" <gi...@apache.org>.
polyzos commented on PR #2997:
URL: https://github.com/apache/fluss/pull/2997#issuecomment-4190777963
LGTM.. thank you for your contribution
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "morazow (via GitHub)" <gi...@apache.org>.
morazow commented on code in PR #2997:
URL: https://github.com/apache/fluss/pull/2997#discussion_r3091983974
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/token/AzureDelegationTokenProvider.java:
##########
@@ -49,15 +46,19 @@ public class AzureDelegationTokenProvider {
public AzureDelegationTokenProvider(String scheme, Configuration conf) {
this.scheme = scheme;
- this.clientId = conf.get(CLIENT_ID);
- this.clientSecret = conf.get(CLIENT_SECRET);
- this.authEndpoint = conf.get(ENDPOINT_KEY);
+ String clientIdKey = AzureFileSystemOptions.CLIENT_ID.key();
+ String clientSecretKey = AzureFileSystemOptions.CLIENT_SECRET.key();
+ String endpointKey = AzureFileSystemOptions.ENDPOINT_KEY.key();
+
+ this.clientId = conf.get(clientIdKey);
+ this.clientSecret = conf.get(clientSecretKey);
+ this.authEndpoint = conf.get(endpointKey);
Review Comment:
I think we can inline the keys in the `get(...)` calls? Since the keys are not used afterwards.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [filesystem][bug] Fix client.fs.* config prefix not stripped before passing to… [fluss]
Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on code in PR #2997:
URL: https://github.com/apache/fluss/pull/2997#discussion_r3093344030
##########
fluss-filesystems/fluss-fs-azure/src/main/java/org/apache/fluss/fs/azure/token/AzureDelegationTokenProvider.java:
##########
@@ -49,15 +46,19 @@ public class AzureDelegationTokenProvider {
public AzureDelegationTokenProvider(String scheme, Configuration conf) {
this.scheme = scheme;
- this.clientId = conf.get(CLIENT_ID);
- this.clientSecret = conf.get(CLIENT_SECRET);
- this.authEndpoint = conf.get(ENDPOINT_KEY);
+ String clientIdKey = AzureFileSystemOptions.CLIENT_ID.key();
+ String clientSecretKey = AzureFileSystemOptions.CLIENT_SECRET.key();
+ String endpointKey = AzureFileSystemOptions.ENDPOINT_KEY.key();
+
+ this.clientId = conf.get(clientIdKey);
+ this.clientSecret = conf.get(clientSecretKey);
+ this.authEndpoint = conf.get(endpointKey);
Review Comment:
Well, endpointKey is used later a couple of times in logs and also in additionInfos, so it's bit verbose, but at least consistent and no surprises. The alternative might be more confusing, so I'd prefer this way tbh
I cleaned up `conf.get` for additionInfos though :)
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org