General create-sibling RF and few enhancements
#1232
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1232 +/- ##
==========================================
+ Coverage 89.01% 89.06% +0.04%
==========================================
Files 235 235
Lines 23688 23728 +40
==========================================
+ Hits 21086 21133 +47
+ Misses 2602 2595 -7
Continue to review full report at Codecov.
|
They are all on the same host!
| raise ValueError("""insufficient information for target creation | ||
| (needs at least a dataset and a SSH URL).""") | ||
| # there is no point in doing anything further | ||
| not_supported_on_windows( |
There was a problem hiding this comment.
This is superfluous. SSHManager instantiation raises this before we get here.
There was a problem hiding this comment.
Cool. Note that I only moved this up....
| sshri = RI(sshurl) | ||
|
|
||
| if not isinstance(sshri, SSHRI) \ | ||
| and not (isinstance(sshri, URL) and sshri.scheme == 'ssh'): |
There was a problem hiding this comment.
While at it: condition should be replaceable by not datalad.support.network.is_ssh() now.
There was a problem hiding this comment.
OK, will do. If "should" means "that works".
There was a problem hiding this comment.
:-) I used it with the entire connection sharing mechanic.
Therefore "should" means: "If I didn't miss an important point".
|
|
||
| # find datasets with existing remotes with the target name | ||
| remote_existing = [p for p in datasets | ||
| if name in datasets[p].repo.get_remotes()] |
There was a problem hiding this comment.
Not sure, but as a hint: If we run into issues here with real data, we might need an AnnexRepo.get_remotes() (overriding the one from GitRepo) in order to filter out special remotes.
There was a problem hiding this comment.
I don't get this. If we have an AnnexRepo instance, this will be called anyways. If we don't have one, we cannot have special remotes. What am I missing?
There was a problem hiding this comment.
You miss, that AnnexRepo currently is just using the method inherited from GitRepo. It's all fine herein, but we might need to change this within AnnexRepo.
There was a problem hiding this comment.
OK, so nothing that needs to go into this change.
There was a problem hiding this comment.
Yes, just wanted to save you time, in case to try with an setup, that would trigger that issue. ;-)
There was a problem hiding this comment.
@bpoldrack an issue for that would be good. This information will soon be lost in this PR otherwise.
Also yield more flexible procedure, removed a whole bunch of test corner cases and reverted back to original behavior
Mainly to see better what parameterization is actually in effect.
The previous one would not work if the root dataset itself were to be skipped.
No test yet, but please play with it
|
FTR: All untested lines correspond to code that was just moved, but was there before. |
|
FTR: This will not see further changes, unless required by something in #1237 |
|
Alrighty, life goes on. Merge. |
Continuation of #1232, focus on add-sibling, create-sibling*, publish interaction
So far, mostly sorting checks to fail as early (and as cheap) as possible, plus some RF to use more common code. And now also various minor fixes and simplifications. Brought back previously disabled tests.
If you are happy about what you see feel free to click the merge button.
create_sibling()andadd_sibling()interaction #1233