github-actions[bot] commented on code in PR #41268:
URL: https://github.com/apache/beam/pull/41268#discussion_r4559676699
##########
sdks/python/apache_beam/io/filesystems.py:
##########
@@ -131,10 +135,14 @@ def get_filesystem(path):
"""
try:
path_scheme = FileSystems.get_scheme(path)
- systems = [
- fs for fs in FileSystem.get_all_subclasses()
- if fs.scheme() == path_scheme
- ]
+ direct_subclasses = FileSystem.__subclasses__()
+ if FileSystems._direct_subclasses_cache != direct_subclasses:
+ by_scheme = {}
+ for fs in FileSystem.get_all_subclasses():
Review Comment:
**🟢 Low**: `FileSystem.get_all_subclasses()` traverses the full inheritance
hierarchy recursively, whereas `FileSystem.__subclasses__()` only checks direct
subclasses of `FileSystem`. If a custom `FileSystem` plugin ever subclasses an
existing `FileSystem` subclass (e.g., `LocalFileSystem` or `GCSFileSystem`)
after the cache is populated, `FileSystem.__subclasses__()` would not change
and the cache would not be invalidated. Additionally, if an intermediate base
class does not implement `scheme()`, calling `fs.scheme()` during cache
population would raise `NotImplementedError` even for unrelated schemes (which
previously would have failed too, so that part is unchanged). This is fine for
all built-in filesystems, though if indirect subclasses are not expected to be
dynamically registered, this check is sufficient.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
