Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0.0] AbstractFileSystem and AbstractVfsComponent are custom #62

Open
boris-petrov opened this issue Oct 10, 2019 · 4 comments
Open

Comments

@boris-petrov
Copy link
Contributor

I saw the following error in our logs:

class com.github.vfss3.S3FileSystem cannot be cast to class org.apache.commons.vfs2.provider.AbstractFileSystem

Coming from here:

commons-vfs2-2.4.1.jar!/org/apache/commons/vfs2/impl/DefaultFileMonitor$FileMonitorAgent.class:423→ fireAllCreate

And I saw that in version 4.0.0 of vfs-s3 there is a custom class AbstractFileSystem and AbstractVfsComponent (at least these two, maybe there are more). This makes vfs-s3 incompatible with utilities provided by commons-vfs like the DefaultFileMonitor.

@abashev
Copy link
Owner

abashev commented Oct 12, 2019

@boris-petrov are you using DefaultFileMonitor only with S3 file systems or is it shared monitor for other types?

@boris-petrov
Copy link
Contributor Author

@abashev - it is shared. We use the same code for all VFS protocols, that's the whole idea of VFS...

I'm not sure why you need these custom classes? Are there still concurrency/other kinds of bugs in the upstream repo that you've fixed? If there are, please open PRs on their side so that all users can benefit.

@abashev
Copy link
Owner

abashev commented Oct 14, 2019

@boris-petrov lol 😀 I tried to submit a few times but even small changes are not going through. That is why I decided not to manage separated commons-vfs fork but to take all required classes inside the project and don't rely on the Apache's project. commons-vfs has really poor design and instead of using interfaces, somewhere inside, they are doing a conversion to a specific class - I can't fix that problem on my end.
I can take DefaultFileMonitor in my project and provide correct, platform-agnostic implementation. But you have to replace all the usages with my class. Is it ok for you?

@boris-petrov
Copy link
Contributor Author

@abashev - true, I've seen how they seem to ignore your PRs, no idea why. They seem to be accepting mine though. 😄 Perhaps I could try? Can you fork the original repo and create a few branches with your commits that I can look at? Do you have some tests that "prove" that your fixes are really needed?

I think it's better to go this way. I agree that commons-vfs is badly designed, believe me, I've spent quite some time on it... but there is nothing we can do about that now. Let's try to fix what we have because a lot of people use that in production. :)

As a last resort we could go the way you suggest, yes, by using a custom DefaultFileMonitor. But I do believe that we can make it work the "right" way.

Thank you for your time!

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

No branches or pull requests

2 participants