Skip to content

Conversation

LuQQiu
Copy link
Collaborator

@LuQQiu LuQQiu commented Mar 11, 2024

Alluxio configuration duplicates in AlluxioClient and Alluxio fsspec AlluxioFileSystem.
To avoid the duplication, put the AlluxioClient in AlluxioClientConfig class.
All the arguments are passed all the way from AlluxioFileSystem -> AlluxioClient -> AlluxioClientConfig as **kwargs.

@LuQQiu
Copy link
Collaborator Author

LuQQiu commented Mar 12, 2024

@ChunxuTang @jja725 PTAL, thanks!

@LuQQiu
Copy link
Collaborator Author

LuQQiu commented Mar 12, 2024

@jja725 @ChunxuTang PTAL, thanks!

etcd_refresh_workers_interval (int, optional): The interval to refresh worker list from ETCD membership service periodically.
All negative values mean the service is disabled.
"""
if not (etcd_hosts or worker_hosts):
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I feel like a lot of these can ust use assert


def shutdown_background_update_ring(self):
if self._etcd_hosts and self._etcd_refresh_workers_interval > 0:
if (
Copy link
Collaborator

Choose a reason for hiding this comment

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

why change to self._config.etcd_hosts is not None

self._options = options
self._hash_node_per_worker = hash_node_per_worker
self._max_attempts = max_attempts
self._config = config
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like we might still want to unwrap the config here like self._etcd_hosts = etcd_hosts and use the member value directly, like what we did in EtcdClient. What do you think

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