Skip to content
This repository has been archived by the owner on Jan 17, 2018. It is now read-only.

Add RemotePath mount option #46

Merged
merged 6 commits into from
Aug 8, 2016
Merged

Add RemotePath mount option #46

merged 6 commits into from
Aug 8, 2016

Conversation

zlatan
Copy link
Contributor

@zlatan zlatan commented Jul 29, 2016

I add an option remotepath which will allow us to use a specific directory in File Share as a volume in the docker container.

@ahmetb
Copy link
Contributor

ahmetb commented Jul 29, 2016

The word remotepath sounds a bit confusing to me. It took me several minutes to understand what it does. Is this part of the SMB/Azure File Storage terminology or did you come up with it?

Also did you test this change?

@zlatan
Copy link
Contributor Author

zlatan commented Aug 1, 2016

RemotePath is part of the terminology of SMB (cf. https://technet.microsoft.com/en-us/library/jj635724(v=wps.630).aspx ).
The code of pull request is tested and work properly.

@ahmetb
Copy link
Contributor

ahmetb commented Aug 2, 2016

@zlatan I just took a major bugfix on the master branch. Do you mind rebasing and ping me when you do?

@zlatan
Copy link
Contributor Author

zlatan commented Aug 5, 2016

@ahmetalpbalkan Rebase is ready.

@@ -298,6 +298,13 @@ func mount(accountName, accountKey, storageBase, mountPath string, options Volum
options.GID = "0"
}
mountURI := fmt.Sprintf("//%s.file.%s/%s", accountName, storageBase, options.Share)
if len(options.RemotePath) != 0 {
if options.RemotePath[0] != '/' {
Copy link
Contributor

Choose a reason for hiding this comment

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

@zlatan I believe you can use path.Join(.. , ..) here to append the slash without checking if it starts with a slash (it could be starting with multiple slashes too).

path.Join("//a.b.com/", "////b") -> //a.b.com/b

@ahmetb
Copy link
Contributor

ahmetb commented Aug 5, 2016

@zlatan mostly looks good, thank you very much. Can you please document it in README.md as well (especially since the terminology is a bit hard to understand in this case)?

@zlatan
Copy link
Contributor Author

zlatan commented Aug 8, 2016

@ahmetalpbalkan README.md was updated, path.Join was used for concatenation of remotepath to mountURI.

@ahmetb
Copy link
Contributor

ahmetb commented Aug 8, 2016

@zlatan thanks for following up. merging.

@ahmetb ahmetb merged commit bc8b8c0 into Azure:master Aug 8, 2016
@zlatan
Copy link
Contributor Author

zlatan commented Aug 9, 2016

@ahmetalpbalkan Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants