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

remote modify: add name param? #3599

Closed
jorgeorpinel opened this issue Apr 6, 2020 · 12 comments · Fixed by #3794
Closed

remote modify: add name param? #3599

jorgeorpinel opened this issue Apr 6, 2020 · 12 comments · Fixed by #3794
Labels
enhancement Enhances DVC feature request Requesting a new feature p3-nice-to-have It should be done this or next sprint

Comments

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Apr 6, 2020

So you can change the name of a remote e.g.

$ dvc remote add myremote some/path
$ dvc remote list
myremote some/path
$ dvc remote modify myremote name supercoolremote
$ dvc remote list
supercoolremote some/path

The workaround rn is to just add the remote again and remove the previous one. But if it has some special configuration then it's more difficult. A better workaroud is to edit the .dvc/config file but relatively advanced.

git remote lets you change a remote name BTW.

@jorgeorpinel jorgeorpinel added the enhancement Enhances DVC label Apr 6, 2020
@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Apr 6, 2020

BTW if you actually run my example command to modify the inexistent name param, it gets written to .dvc/config, which causes an error output whenever you try to change the config later:

ERROR: configuration error - config file error:
extra keys not allowed @ data['remote']['myremote']['name']

You have to manually fix the config file to fix this.

@jorgeorpinel jorgeorpinel added the bug Did we break something? label Apr 6, 2020
@shcheklein
Copy link
Member

which causes an error output whenever you try to change the config later:

(Yep, I hit the same issue here #3552 - quite annoying one :) )

A better workaroud is to edit the .dvc/config file but relatively advanced.

not sure this is very advanced to be honest

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Apr 6, 2020

Not advanced in terms of skill, but you need to know that remote config is in that file. It won't be obvious knowledge for DVC beginners. Also for gdrive remotes the file in .dvc/tmp/...json would also need to be changed manually, I think (if it already exists).

@shcheklein
Copy link
Member

Also for gdrive remotes the file in .dvc/tmp/...json would also need to be changed manually, I think (if it already exists).

it doesn't contain the remote name yet. With GDrive multiple remotes are not supported effectively yet.

It won't be obvious knowledge for DVC beginners.

it's a good question. I think for people familiar with command line, git, etc it should be more or less intuitive that there is a config somewhere.

@jorgeorpinel
Copy link
Contributor Author

OK. No strong opinion. I would err on the side of adding this which I'm assuming should be pretty easy, since git remote allows you to do it, so you may expect this if you're used to Git. + the other reasons outlines. Feel fee to close this though, up to you! Thanks

@shcheklein shcheklein added feature request Requesting a new feature p3-nice-to-have It should be done this or next sprint and removed bug Did we break something? labels Apr 6, 2020
@karajan1001
Copy link
Contributor

karajan1001 commented Apr 23, 2020

I'd like to look into it. @jorgeorpinel How about make it dvc remote rename <old> <new> following the similar command in Git git remote rename <old> <new>.

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Apr 23, 2020

Good idea, indeed that's how it works on Git. But since we already have a modify subcommand to change all possible remote config I'd prefer to add a name config param for that.

@shcheklein
Copy link
Member

It does not hurt to add both? Is there some modify command analog in GIt?

@jorgeorpinel
Copy link
Contributor Author

Not really. They have a subcommand for each remote config param you can change: https://git-scm.com/docs/git-remote

@efiop
Copy link
Contributor

efiop commented Apr 23, 2020

name is ugly. dvc remote rename is much better.

@efiop
Copy link
Contributor

efiop commented Apr 23, 2020

Plus name complicates the argument resolution, I really don't like that and don't think it is worth bothering with. Let's just introduce dvc remote rename as suggested by @karajan1001 .

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Apr 23, 2020

I have no strong opinion. Either introduce name to https://dvc.org/doc/command-reference/remote/modify#available-parameters-for-all-remotes (implies less maintenance both in core code and docs) or introduce rename subcommand.

karajan1001 added a commit to karajan1001/dvc that referenced this issue May 13, 2020
1. add new command `dvc remote rename <old> <new>`.
2. add tests for this new command.
efiop pushed a commit that referenced this issue May 20, 2020
* fixed #3599

1. add new command `dvc remote rename <old> <new>`.
2. add tests for this new command.

* Update dvc/command/remote.py

Co-authored-by: Peter Rowlands (변기호) <[email protected]>

* Change about code review

* For deepsource

* Update dvc/command/remote.py

Co-authored-by: Peter Rowlands (변기호) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC feature request Requesting a new feature p3-nice-to-have It should be done this or next sprint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants