Fix RIA/ORA publication dependencies #5415
Conversation
There was a problem hiding this comment.
My superficial comment is re performance: Can it happen that we clone from store X, and we have an ORA URL pointing to the same X, but we would make a mistake when we configure a publication dependency. I will like the additional query for a UUID makes things slower, but not more reliable.
I think you misread things (or I made a mistake). Intention is to only check the uuid if we have no URL match. |
Codecov Report
@@ Coverage Diff @@
## master #5415 +/- ##
==========================================
+ Coverage 89.48% 89.51% +0.03%
==========================================
Files 296 296
Lines 41800 41907 +107
==========================================
+ Hits 37404 37513 +109
+ Misses 4396 4394 -2
Continue to review full report at Codecov.
|
bpoldrack
force-pushed
the
fix-5396
branch
2 times, most recently
from
fe51741
to
21ced60
Compare
datalad/core/distributed/clone.py
Outdated
| if r['annex-uuid'] == org_uuid] | ||
| if uuid_matching_remotes: | ||
| # we can't have multiple | ||
| assert len(uuid_matching_remotes) == 1 |
There was a problem hiding this comment.
Challenge accepted :)
assertion trigger
This snippet depends on having datalad's test ssh target as datalad-test in your ssh_config.
set -eu
cd "$(mktemp -d /tmp/dl-sameas-XXXXXXX)"
mkdir store
store="$(pwd)/store"
datalad create a
(
cd a
datalad create-sibling-ria -s s1 ria+ssh://datalad-test"$store"
git annex initremote s1-store-local --sameas=s1-storage \
type=external externaltype=ora autoenable=true \
url=ria+file://"$store"
datalad push --to=s1
)
dsid=$(git -C a config --file=.datalad/config datalad.dataset.id)
datalad clone ria+file://"$store#$dsid" b || :
(
cd b
echo
datalad -f json siblings query | \
jq 'select(."annex-externaltype" == "ora") |
{name, "annex-externaltype", "annex-uuid"}'
echo
git config --get-regexp remote.s1
)[...]
[ERROR] AssertionError() [clone.py:postclonecfg_ria:888] (AssertionError)
{
"name": "s1-store-local",
"annex-externaltype": "ora",
"annex-uuid": "beee8cb5-090a-4f4c-b121-e4342f89ea73"
}
{
"name": "s1-storage",
"annex-externaltype": "ora",
"annex-uuid": "beee8cb5-090a-4f4c-b121-e4342f89ea73"
}
remote.s1-store-local.annex-externaltype ora
remote.s1-store-local.annex-uuid beee8cb5-090a-4f4c-b121-e4342f89ea73
remote.s1-store-local.annex-config-uuid a2e7a24e-8fe1-42d6-9358-6be571f813fc
remote.s1-store-local.annex-cost 100.0
remote.s1-store-local.annex-availability GloballyAvailable
remote.s1-storage.annex-externaltype ora
remote.s1-storage.annex-uuid beee8cb5-090a-4f4c-b121-e4342f89ea73
remote.s1-storage.annex-cost 200.0
remote.s1-storage.annex-availability GloballyAvailable
remote.s1-storage.annex-ignore false
remote.s1-storage.skipfetchall true
--sameas is probably pretty rare and even rarer with ORA special remotes, but it's still probably worth filtering out sameas remotes here.
There was a problem hiding this comment.
Amend and force-pushed.
There was a problem hiding this comment.
Thanks.
The PR in general looks reasonable to me, though given how little I know about ria/ora, that doesn't count for much.
Setting the publication dependency for 'origin' on an ORA remote in postclone routine didn't properly account for multiple accessible ORA remotes or the fact that none of them may be associated with the RIA store we are cloning from. Now, first check for an ORA remote with the same store URL registered and if there is none check whether the store has a remote's uuid stored in the .git/config of the dataset. If we have that remote enabled, set the dependency on it. (Closes datalad#5396)
Setting the publication dependency for 'origin' on an ORA remote in
postclone routine didn't properly account for multiple accessible ORA
remotes or the fact that none of them may be associated with the RIA
store we are cloning from.
Now, first check for an ORA remote with the same store URL registered
and if there is none check whether the store has a remote's uuid stored
in the .git/config of the dataset. If we have that remote enabled, set
the dependency on it.