-
Notifications
You must be signed in to change notification settings - Fork 113
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
Cs3 share manager #2626
Cs3 share manager #2626
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
aab0ac6
to
7bceda9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found some smaller ones. Looks good in general!
pkg/share/manager/cs3/cs3.go
Outdated
return nil, err | ||
} | ||
|
||
fn := path.Join("shares", share.Id.OpaqueId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw a few places where we hardcode path segments like this. I wonder if we should not have a central place where we define these instead of hardcoding.
Maybe not really part of this PR..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dragotin I just pushed a commit which changes the remaining places to also make use of shareFilename()
which encapsulates the knowledge about the directoy layout.
Not sure if that's what you meant, but I don't think we need to move this to a central place inside the repository as the layout is only relevant to the share manager itself.
This PR introduces a new share manager which uses the new metadata storage backend.