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

Jump to bottom

BF: RIA cloning #5255

Merged
merged 4 commits into from Dec 17, 2020
Merged

BF: RIA cloning #5255

merged 4 commits into from Dec 17, 2020

Conversation

bpoldrack
Copy link
Member

Fixes #5186

Sits on top of PR #5254 due to #5253, which is not the same but a somewhat related issue.

Despite @mih's hesitance, I'd like to support RIA stores without any ORA remote. Since an ID based structure for standard, bare annex repos is perfectly valid and requires all the same stuff (URL resolution for example). In such a case this fix would not fix but sabotage the intended behavior. With that in mind, this one-liner is a function, that can be enhanced to first query the store for a to be introduced dataset level layout, that would disable the annex-ignore setting.

kyleam
kyleam reviewed Dec 16, 2020
View reviewed changes
Copy link
Contributor

@kyleam kyleam left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good to me aside from the import issue I noted.

Even if we expect 0.14 out before another maintenance release, I'm guessing you intended to choose maint rather than master as the base for this PR?

datalad/core/distributed/clone.py
@@ -918,9 +933,6 @@ def postclonecfg_annexdataset(ds, reckless, description=None):
yield from configure_origins(ds, ds)


_handle_possible_annex_dataset = postclonecfg_annexdataset
Copy link
Contributor

Choose a reason for hiding this comment

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

There's also an import in the deprecated datalad.distribution.clone.

@codecov
Copy link

codecov bot commented Dec 16, 2020 •
edited

Codecov Report

Merging #5255 (9e8413c) into maint (ee25838) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##            maint    #5255   +/-   ##
=======================================
  Coverage   90.43%   90.44%           
=======================================
  Files         294      294           
  Lines       40936    40962   +26     
=======================================
+ Hits        37021    37047   +26     
  Misses       3915     3915
Impacted Files Coverage Δ
datalad/distribution/clone.py 100.00% <ø> (ø)
datalad/core/distributed/clone.py 90.00% <100.00%> (+0.05%) ⬆️
datalad/core/distributed/tests/test_clone.py 96.91% <100.00%> (+0.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee25838...9e8413c. Read the comment docs.

@bpoldrack
Copy link
Member Author

Re maint: True!

Re import: Indeed. How did git-grep not find it? Anyway, will remove, too.

Thx!

@bpoldrack
Copy link
Member Author

crippledFS failures look unrelated. Do they seem famililar, @datalad/developers ?

@kyleam
Copy link
Contributor

kyleam commented Dec 16, 2020

crippledFS failures look unrelated. Do they seem famililar, @datalad/developers ?

At least the test_cached_dataset one is a regular. The build is red frequently enough that I'm guilty of not paying much attention to it (especially when I can't confirm with tools/eval_under_testloopfs locally).

@mih
Copy link
Member

mih commented Dec 16, 2020 •
edited

Thanks! I agree that it should go into maint. Other than that it looks good to me. The crippled FS failures are old friends.

@bpoldrack
TST: Proof datalad#5186
b186fa5
@bpoldrack
BF: annex-ignore RIA sources
e8c21e0 
The bare repos in a RIA store are intended to not have a local annex
they know of, although there is an annex/ dir for access via ORA special
remote as well as for enabling workflows using ephemeral clones
symlinking into that location.

However, if we clone from local FS, annex-init on the clone may also
initialize origin, posing a danger for messing up availability info.
Prevent that by setting annex-ignore before we initialize the clone (we
used to do that afterwards anyway).

(Closes datalad#5186)
@bpoldrack
Remove obsolete alias
93e0746
@bpoldrack bpoldrack changed the base branch from master to maint December 17, 2020 06:12
@mih mih merged commit 79c31d8 into datalad:maint Dec 17, 2020
@mih
Copy link
Member

mih commented Dec 17, 2020

Thx!

@mih mih deleted the bf-5168 branch December 17, 2020 07:12
@mih
Copy link
Member

mih commented Dec 17, 2020

I was too quick and did not acknowledge Remove obsolete alias. This isn't desired in maint and largely unnecessary in master (already removed file).

I have cleanup up maint by removing the commit and then merged into master.

@yarikoptic yarikoptic added this to the 0.13.7 milestone Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kyleam kyleam kyleam left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.13.7
Development

Successfully merging this pull request may close these issues.

datalad clone "ria+file:/some/store/#~myds" leads to undesired establishment of an "annex"
4 participants
@bpoldrack @kyleam @mih @yarikoptic