Skip to content

Conversation

@drexler-sky
Copy link

@drexler-sky drexler-sky commented Jul 14, 2025

This PR introduces a new get_hdfs_by_full_path_with_config method to HdfsFs, enabling programmatic configuration of HDFS connections through a HashMap of key-value pairs.

@drexler-sky
Copy link
Author

@yahoNanJing Could you please take a look at this PR? Thanks!

@drexler-sky
Copy link
Author

also cc @Kontinuation @comphead @parthchandra

src/hdfs.rs Outdated
Comment on lines 193 to 196
pub fn connect_with_config(
nn: &str,
config: &HashMap<String, String>,
) -> Result<Arc<Self>, HdfsErr> {

Choose a reason for hiding this comment

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

The main entry for getting an Arc<HdfsFs> is get_hdfs_by_full_path. It caches already connected file system handles and will reuse them when getting another HdfsFs using uri with cached host. HdfsFs handles were never closed, since the Rust binding does not call hdfsDisconnect at all.

The caller of connect_with_config has to deal with caching and reusing already connected HdfsFs, otherwise we'll leave lots of connected Hadoop FileSystem objects in the JVM.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your comment! I’ve updated the implementation to use get_hdfs_by_full_path_with_config, so it now follows the same caching logic as get_hdfs_by_full_path and reuses existing HdfsFs handles. Let me know if there’s anything else I should adjust.

@drexler-sky drexler-sky changed the title feat: Add connect_with_config for configurable HDFS connections feat: Add get_hdfs_by_full_path_with_config for configurable HDFS connections Jul 21, 2025
@drexler-sky
Copy link
Author

Gentle ping @yahoNanJing, when you have a chance, could you please take a look at this?

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.

2 participants