Is it a good idea that allow user pass the HDFS home to `ClientBuilder`?

[PeterlitsZo]

Currently, Configuration::get_conf_dir reads the HADOOP_CONF_DIR (or HADOOP_HOME) environment variable to load configurations.

What if we allowed users to pass the HDFS configuration directory as an argument? This would be useful for scenarios where we need to access multiple HDFS clusters, each with its own installation path.

While we can currently use set_env to set HADOOP_HOME before creating a client, I think it would be cleaner if we could directly specify the desired path to the ClientBuilder.

[PeterlitsZo]

If you want, I or my friend can send a PR, but not sure which API would be better.

[Kimahriman]

Hmm I would expect different clusters would still be in a single config file you could access by name, but I'm fine adding this in as an option and just fallback to the env vars if not defined in the builder

[Lostlitus]

Hi @Kimahriman, currently I try to add a secure namesevice to my ViewFS together with other non-secure namservices, but command like hdfs dfs -ls / says secure ns and non-secure ns cannot be mixed in the config. So I think it would be better to put the secure one's config in a separate path like /opt/hadoop-secure, and for cli users, they can export HADOOP_HOME to access it. Similarly, for Rust user, they can build a client with default config path and another client with the secure one's path.

Any advice for my ideas? I find that other Rust HDFS client libs do not support this feature. Maybe there exists some workaround, like manually setting env in code before building client. But std::env::set_var is unsafe function now and using it for building client looks quite hacky...

[Kimahriman]

Hey sorry I forgot to get back to this. I don't think there's even a way to do this in the Java client but if you wanted to try to add something like this I'd imagine:

• Create a ConfigurationBuilder struct for building configs instead of the two constructors that exist. One method with_conf_dir can set the config dir and override the env vars it falls back to
• Add a method on the ClientBuilder for with_conf_dir that sets that on the underlying config object on build

[PeterlitsZo]

What do you prefer? I think ConfigurationBuilder sounds better. This object can be used to build Configuration from FS, or hashmap, etc.

[Lostlitus]

Hey @Kimahriman never mind, the lib is great and I have good experience using it.

I don't think there's even a way to do this in the Java client

Yes, I found the Java guys uses the workaround similar to std::env::set_var one. Since the lib here has the ability to read and parse the configuration by itself, I think adding a way to pass the path in run time is easier than those Java or Java wrapper libs.

• Create a ConfigurationBuilder struct for building configs instead of the two constructors that exist. One method with_conf_dir can set the config dir and override the env vars it falls back to
• Add a method on the ClientBuilder for with_conf_dir that sets that on the underlying config object on build

Looks reasonable to me. Maybe Configuration's new method is enough for building the configs? It should work with small adjustments. Adding ConfigurationBuilder could be taken into consideration when much more complex feature is needed.

I added a small patch to this feature here. If it looks good to you I would send a PR then. Or you want some modifications or a big refactoring, I'd be glad to adjust it.

[PeterlitsZo]

Maybe Configuration's new method is enough for building the configs?

Using a builder pattern works for me. Since this is a library, it's important to maintain a stable API — and a builder can really help with that by minimizing breaking changes in the future.

EDITED: After looking into it further, I noticed that even though we use pub, the Configuration struct isn't actually being exported (maybe using pub(crate) is better). That makes me wonder if a simple new method might be enough. Still, I personally lean toward the builder pattern for its flexibility — but I'm happy to hear other thoughts on this.

[Kimahriman]

Yeah since it's not public it really doesn't matter how the Configuration change is handled, it could just be a single new method that takes options for any of the optional things (config map, config dir)

[Kimahriman]

I added a small patch to this feature here. If it looks good to you I would send a PR then. Or you want some modifications or a big refactoring, I'd be glad to adjust it.

Yeah looks generally fine

[Kimahriman]

Resolved by #284