Skip to content

add cache.shared config reference and use case#971

Merged
shcheklein merged 2 commits into
masterfrom
497
Feb 4, 2020
Merged

add cache.shared config reference and use case#971
shcheklein merged 2 commits into
masterfrom
497

Conversation

@efiop
Copy link
Copy Markdown
Contributor

@efiop efiop commented Feb 4, 2020

Fixes #497

You may disregard these recommendations if you used the Edit on GitHub button from dvc.org to improve a doc in place.

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.

🐛 Please make sure to mention Fix #issue (if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.

Please chose to
allow us
to edit your branch when creating the PR.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

@efiop
Copy link
Copy Markdown
Contributor Author

efiop commented Feb 4, 2020

One more user asked about it on discord and so, as usual, I went to check the docs and noticed that there was nothing about it because I've just created a ticket a year ago and didn't send a PR. @jorgeorpinel you've spoiled me with great docs! 😄 So I've decided to send a PR right away, so I don't forget about it, will get back to it soon.


- `cache.shared` - permissions for newly created or downloaded cache files and
directories. The default permissions are `0o664`(rw-r--r--) for files and
`0o755`(rwxr-xr-x) for directories. The only accepted value right now is
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is presenting permissions in such way clear enough? I'm not sure if it is too hard to understand, maybe we are better describing it with words to make it as friendly as possible?

```dvc
$ sudo find /path/to/dvc-cache -type f -exec chmod 0664 {} \;
$ sudo find /path/to/dvc-cache -type d -exec chmod 0775 {} \;
$ sudo chown -R myuser:ourgroup /path/to/dvc-cache/
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Needs clarification that the user needs to use his username and his group, that he shares with his team. Also sudo is probably also something that we need to describe better, as it might require asking devops guys or something, not sure if usually DS teams have sudo access on their servers.

Comment on lines +59 to +65
And tell DVC to set group permissions on the newly created/downloaded cache
files:

```dvc
$ dvc config cache.shared group
```

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reason why it wasn't needed before is that I've assumed that people will just create their files with proper group permissions and so our cache files won't need any changes. That was clearly too much to ask, so we now do it explicitly.

- `cache.shared` - permissions for newly created or downloaded cache files and
directories. The default permissions are `0o664`(rw-r--r--) for files and
`0o755`(rwxr-xr-x) for directories. The only accepted value right now is
`group`, which makes dvc use `0o664`(rw-rw-r--) for files and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor: dvc -> DVC

Copy link
Copy Markdown
Contributor

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

just a minor change and I'll merge it :) looks good to me

@shcheklein shcheklein merged commit edf66a5 into master Feb 4, 2020
@shcheklein shcheklein temporarily deployed to dvc-landing-497-zu8xmctji75pvl February 4, 2020 15:25 Inactive
@efiop efiop deleted the 497 branch February 4, 2020 17:19
@jorgeorpinel
Copy link
Copy Markdown
Contributor

Thanks @efiop!

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

Successfully merging this pull request may close these issues.

cmd ref: add info about cache.shared group and available setting, also in use-cases

3 participants