Skip to content

Load configuration directory in run time#284

Merged
Kimahriman merged 3 commits intoKimahriman:masterfrom
Lostlitus:load-config-dir-in-run-time
Feb 20, 2026
Merged

Load configuration directory in run time#284
Kimahriman merged 3 commits intoKimahriman:masterfrom
Lostlitus:load-config-dir-in-run-time

Conversation

@Lostlitus
Copy link
Contributor

Followup for #283

The choices of configuration now:

1. From `HADOOP_CONF_DIR`.
2. From `HADOOP_HOME`.
3. From passed kv pair.

In the case that users want to set configuration in run time(say having
multiple HDFS clients with different configurations), they have two
workarounds:

1. Set the environment variables manually via `std::env::set_var`.
2. Load configuration manually and build client by
   `client::ClientBuilder::with_config`.

The first one introduces unsafe code and the latter is much more
troublesome.

Since the lib has the ability to read and parse the configuration by itself,
I think adding an option for `client::ClientBuilder` to override the path
read from environment variables is reasonable.

Now `client::ClientBuilder` accepts config path via `with_config_dir`.
And the path would be passed to `common::Configuration` to generate the
final configs together with those passed by `with_config`.

Small refactoring is made to `client::ClientBuilder`, decoupling the
configuration reading and parsing code.
@Lostlitus
Copy link
Contributor Author

Ping @Kimahriman @PeterlitsZo 👀
I tried to pass all the tests and to make my lsp stop warning, so some reasonable fixes is added.

Copy link
Owner

@Kimahriman Kimahriman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few comments but looks good

@Lostlitus Lostlitus force-pushed the load-config-dir-in-run-time branch from 7f2591c to 467876c Compare February 19, 2026 03:21
@PeterlitsZo
Copy link
Contributor

I add some comments here. Very good work 😃 thank you.

@Lostlitus Lostlitus force-pushed the load-config-dir-in-run-time branch 2 times, most recently from 6934854 to 15bf215 Compare February 19, 2026 11:57
@Kimahriman
Copy link
Owner

Can you add this to the Python side as well? Should be fairly straightforward, don't worry about adding any tests

@Lostlitus
Copy link
Contributor Author

Looks like a build for MiniDfs is needed to generate rust/target/test? Should I build it in the unit test, or add a config directory under rust/test with minimal configs, or just move this test back to integration test?

@Lostlitus
Copy link
Contributor Author

OK, I try the python one

Besides, fix some small problem:

1. Typo and format cases in README.md.
2. compile error in `rust/build.rs`.
@Lostlitus Lostlitus force-pushed the load-config-dir-in-run-time branch from 15bf215 to 40ab816 Compare February 19, 2026 18:44
@Lostlitus
Copy link
Contributor Author

It seems that python one only have test for with_url case. If test code is required for this feature, I guess both with_config and with_config_dir cases are needed? Like adding pytest fixture for client with configs and config directory

@Kimahriman Kimahriman merged commit 72ac1cc into Kimahriman:master Feb 20, 2026
9 checks passed
@Lostlitus Lostlitus deleted the load-config-dir-in-run-time branch February 21, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants