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

feat: FileSystemProvider::readAttributes faked posix support #1067

Merged
merged 1 commit into from Dec 9, 2022

Conversation

nielsbasjes
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

This is the follow up patch after #1066 to add fake posix support in FileSystemProvider::readAttributes.
Important to note is that the values returned in this patch are all fixed and fake.

The main goal of this patch is to have minimal support for the posix attributes because some existing libraries fail if a filesystem does not support this at all. By adding faked support many of these libraries become usable in combination with cloud storage with minimal effort.

I expect that after this some may want to add a more accurate mapping of the attributes.

Fixes #1062 ☕️

@nielsbasjes nielsbasjes requested a review from a team as a code owner December 5, 2022 16:47
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: storage Issues related to the googleapis/java-storage-nio API. labels Dec 5, 2022
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: l Pull request size is large. labels Dec 5, 2022
@BenWhitehead
Copy link
Collaborator

Thanks for opening this PR! I'm reaching out to some teammates to get their input on any gotchas when emulating posix over a gcs bucket.

@nielsbasjes
Copy link
Contributor Author

nielsbasjes commented Dec 5, 2022

Thanks for opening this PR! I'm reaching out to some teammates to get their input on any gotchas when emulating posix over a gcs bucket.

Yes.
In general a GCS bucket cannot do everything a posix filesystem can do.
This PR is not trying to fully emulate posix, only faking the attributes regarding the ownership and permissions. It is really only a view of the attributes.
Having this is enough to get several libraries working in conjunction with a GCS bucket that expect this information in a posix view. Like I ran into here: https://github.com/nielsbasjes/MinimalSftpCloudStorage

So I my view the worst that can happen is that the expected operation fails because the returned fake permission attributes were incorrect (i.e. not allowed to read/write).

@BenWhitehead BenWhitehead added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 5, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 5, 2022
Copy link
Collaborator

@BenWhitehead BenWhitehead left a comment

Choose a reason for hiding this comment

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

The folks I reached out to didn't have any objections to the approach taken hear.

Thanks for the contribution!

@BenWhitehead BenWhitehead added the owlbot:ignore instruct owl-bot to ignore a PR label Dec 9, 2022
@BenWhitehead BenWhitehead merged commit b813ccc into googleapis:main Dec 9, 2022
gcf-merge-on-green bot pushed a commit that referenced this pull request Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage-nio API. owlbot:ignore instruct owl-bot to ignore a PR size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Java Storage NIO: FileSystemProvider::checkAccess fails on '/' with StorageException
3 participants