BF: Remove create-sibling-gitlab's hierarchy layout, make collection the default by jsheunis · Pull Request #7410 · datalad/datalad

jsheunis · 2023-06-06T10:46:10Z

~~This PR:~~

~~addresses Fix hierarchy layout in create-sibling-gitlab #7409 and related issues~~
~~applies a small change to remove the discrepancy between the docs description of the hierarchy layout and the actual implementation.~~
~~adds adjustments for tests~~

The result is that each dataset name in a nested structure, including the top-level dataset, will map onto a gitlab group, and the associated dataset repositories will be created as a projects inside these nested groups.

An additional requirement is that a user needs to create a top-level group via the Gitlab UI (this cannot be done via the API) and this group needs to be provided as the base of the value in the --project argument, e.g. newgroup/mydatasets.

EDIT by adina: This PR started out trying to fix the hierarchy layout of create-sibling-gitlab, but eventually discovered the same problems others have discovered at several different times in the past. See #7411 and #7409.
Now, this PR implements a suggestion that has been made by Kyle already shortly after the original create-sibling-gitlab feature was merged: Make collection the default layout. The need for this arose because the hierarchy layout has a number of edge cases that make it break when it operates on hierarchies of datasets (see #7411 and #7409 as well for details). Rather than keeping the hierarchy layout around in a dysfunctional state, this PR removes it entirely, and adjusts documentation and tests accordingly. I would argue that there is little use if we keep it - it has been broken since it existed for recursive operations, and in non-recursive operations, the outcome that the collection layout provides is identical to what hierarchy would provide.

This commit applies a small change to remove the discrepancy between the docs description of the hierarchy layout and the actual implementation. The result is that each dataset name in a nested structure, including the top-level dataset, will map onto a gitlab group, and the associated dataset repositories will be created as a projects inside these nested groups. An additional requirement is that a user needs to create a top-level group via the Gitlab UI (this cannot be done via the API) and this group needs to be provided as the base of the value in the --project argument, e.g. 'newgroup/mydatasets'.

…gitlab-hierarchy-fix

adswa · 2023-06-06T11:02:15Z

I rebased your changes to sit on top of #7407

adswa · 2023-06-06T13:10:18Z

Hold off reviews or merges, I found an unrelated bug I'd like to slay in this PR, too

This is a fix for datalad#7411. In a dataset hierarchy with subdatasets in subdirectories, the hierarchy layout of create-sibling-gitlab will keep path separators in the project path. This is problematic, because for GitLab, this makes it look like the subdirectory needs to be a pre-existing group. For example: [DS~0] /home/me/dl-101/DataLad-101 ├── recordings/ │ └── [DS~1] longnow/ will create a project path like this: <gitlab-instance>/<gitlab-group>/recordings/longnow/project DataLad will attempt to create the group 'longnow', but won't consider creating 'recordings'. Thus, the action will fail due to a missing parent level group that was never meant to be created. With this change, the project path becomes <gitlab-instance>/<gitlab-group>/longnow/project I.e., subdirectories are stripped.

Reasoning is in datalad#7411

codecov · 2023-06-07T08:45:33Z

Codecov Report

Patch coverage: 79.31% and project coverage change: +2.86 🎉

Comparison is base (4750981) 88.68% compared to head (f3b704e) 91.54%.

❗ Current head f3b704e differs from pull request most recent head 8510b5c. Consider uploading reports for the commit 8510b5c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7410      +/-   ##
==========================================
+ Coverage   88.68%   91.54%   +2.86%     
==========================================
  Files         327      325       -2     
  Lines       44703    43333    -1370     
  Branches     5948     5804     -144     
==========================================
+ Hits        39645    39670      +25     
+ Misses       5043     3648    -1395     
  Partials       15       15

Impacted Files	Coverage Δ
datalad/support/gitrepo.py	`91.95% <ø> (ø)`
datalad/ui/progressbars.py	`79.51% <50.00%> (+2.98%)`	⬆️
datalad/local/wtf.py	`78.99% <57.69%> (+1.25%)`	⬆️
datalad/distributed/create_sibling_gitlab.py	`68.90% <100.00%> (ø)`
datalad/distributed/drop.py	`96.83% <100.00%> (+0.05%)`	⬆️
...ad/distributed/tests/test_create_sibling_gitlab.py	`98.59% <100.00%> (+0.09%)`	⬆️
datalad/distributed/tests/test_drop.py	`100.00% <100.00%> (ø)`

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

jsheunis

Some notes:

Made a very minor doc edit
I've tested it with multiple nested datasets, within subdirectories, and the collection and flat layouts seem to be working as expected
The tests all pass locally
While reviewing this I noticed some issues with the path separator code, but I have moved those comments over to #7407

Improve changelog Update changelog

Co-authored-by: Stephan Heunis <jsheunis@gmail.com>

mih · 2023-06-09T07:44:10Z

Given the time pressure imposed by upcoming events that require this command to work well, I have taken this changeset (plus #7407) and introduced it as a patch in datalad-next: datalad/datalad-next#413

It may be interesting to absorb the RF of the gitlab tests from that PR, which removes nose-residuals and switches to better use of pytest paradigms. The commit cannot be taken 1:1, but needs an adjustment of monkeypatch targets.

Import create-sibling-gitlab patch from datalad/datalad#7410

adswa · 2023-06-12T06:38:59Z

woah, I haven't seen this much green in a while!

adswa · 2023-06-12T06:55:26Z

btw I have created a dedicated walk-through (relies on this PR) in the handbook: https://datalad-handbook--963.org.readthedocs.build/en/963/basics/101-139-hostingservices.html#creating-a-sibling-on-gitlab

* origin/master: [skip ci] Update docs/source/changelog.rst [skip ci] Update CHANGELOG [gh-actions](deps): Bump con/tributors from 0.0.21 to 0.1.1 Use getpwd() instead of Path.cwd() to account for $PWD (symlinked paths) OPT: parse_gitconfig_dump() origin path processing [release-action] Autogenerate changelog snippet for PR 7050 ENH: check file size on the filesytem for relaxed annexed files with content BF: status - report annexed files even if no bytesize known Use pytest.skip instead of raise SkipTest in the added test [release-action] Autogenerate changelog snippet for PR 7422 BF/RF(TST): skip test_system_ssh_version if no ssh found + split parsing into separate test ENH(TST): test that Runner raises FileNotFoundError if binary does not exist remove unused in the test @with_tempfile [release-action] Autogenerate changelog snippet for PR 7418 Do not map (leave as is) trailing \/ in github URLs. [release-action] Autogenerate changelog snippet for PR 7412 Use `sphinx_autodoc_typehints` [release-action] Autogenerate changelog snippet for PR 7388 (codespelled) RF: Issue a warning while minting annex key but getting KeyError BF: make addurls tollerate the case that not all rows have all metadata fields

qlty-cloud-legacy · 2023-06-13T18:56:37Z

Code Climate has analyzed commit 8510b5c and detected 0 issues on this pull request.

View more on Code Climate.

yarikoptic · 2023-06-13T18:57:14Z

master is now up to date with fresh maint release and is 0.18.5-69-g6d5278172 . I have merged it in, and added release label. If tests are all good -- someone hit "Merge" and see what happens ;)

adswa · 2023-06-14T08:23:51Z

Everything is green! I'm hitting merge 💥

adswa · 2023-06-14T08:36:29Z

I think it didn't do anything...

yarikoptic-gitmate · 2023-06-14T14:55:17Z

PR released in 0.19.0

BF: Remove create-sibling-gitlab's hierarchy layout, make collection the default

jsheunis and others added 3 commits June 6, 2023 11:49

update tests related to hierarchy layout

c3387ff

Merge remote-tracking branch 'gh-jsheunis/gitlab-hierarchy-fix' into …

0770727

…gitlab-hierarchy-fix

adswa added semver-minor Increment the minor version when merged CHANGELOG-missing When a PR's description does not contain a changelog item, yet. labels Jun 6, 2023

github-actions Bot removed the CHANGELOG-missing When a PR's description does not contain a changelog item, yet. label Jun 6, 2023

adswa reviewed Jun 6, 2023

View reviewed changes

Comment thread changelog.d/pr-7410.md Outdated

adswa added 3 commits June 6, 2023 15:46

Remove the hierarchy layout, and make collection the new default

6b5a296

Reasoning is in datalad#7411

Mention that users need to create top-level groups

f70d3bd

adswa mentioned this pull request Jun 7, 2023

Gitlab datalad-handbook/book#963

Merged

adswa marked this pull request as ready for review June 7, 2023 10:16

adswa changed the title ~~BF: gitlab hierarchy fix~~ BF: Remove create-sibling-gitlab's hierarchy layout, make collection the default Jun 7, 2023

adswa reviewed Jun 7, 2023

View reviewed changes

Comment thread changelog.d/pr-7410.md Outdated

jsheunis commented Jun 7, 2023

View reviewed changes

Comment thread datalad/distributed/create_sibling_gitlab.py Outdated

DataLad Bot and others added 2 commits June 9, 2023 08:15

generate changelog snippet for PR 7410

8110069

Improve changelog Update changelog

Fix typo

7c83f4a

Co-authored-by: Stephan Heunis <jsheunis@gmail.com>

adswa force-pushed the gitlab-hierarchy-fix branch from 9e50e41 to 7c83f4a Compare June 9, 2023 06:16

mih added a commit to mih/datalad-next that referenced this pull request Jun 9, 2023

Import create-sibling-gitlab patch from datalad/datalad#7410

f73e90e

mih added a commit to datalad/datalad-next that referenced this pull request Jun 9, 2023

Merge pull request #413 from mih/gitlab

977aec4

Import create-sibling-gitlab patch from datalad/datalad#7410

Merge branch 'master' into gitlab-hierarchy-fix

f3b704e

yarikoptic added the release Create a release when this pr is merged label Jun 13, 2023

adswa merged commit 879a262 into datalad:master Jun 14, 2023

adswa mentioned this pull request Jun 14, 2023

Merge master into maint for a release #7428

Merged

adswa added a commit to adswa/datalad that referenced this pull request Jun 16, 2023

Merge pull request datalad#7410 from jsheunis/gitlab-hierarchy-fix

3b81272

BF: Remove create-sibling-gitlab's hierarchy layout, make collection the default

This was referenced Jun 16, 2023

Fix hierarchy layout in create-sibling-gitlab #7409

Closed

create-sibling-gitlab: hierarchy layout can not handle subdatasets in subdirectories #7411

Closed

Conversation

jsheunis commented Jun 6, 2023 • edited by adswa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adswa commented Jun 6, 2023

Uh oh!

Uh oh!

adswa commented Jun 6, 2023

Uh oh!

codecov Bot commented Jun 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

jsheunis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mih commented Jun 9, 2023

Uh oh!

adswa commented Jun 12, 2023

Uh oh!

adswa commented Jun 12, 2023

Uh oh!

qlty-cloud-legacy Bot commented Jun 13, 2023

Uh oh!

yarikoptic commented Jun 13, 2023

Uh oh!

adswa commented Jun 14, 2023

Uh oh!

adswa commented Jun 14, 2023

Uh oh!

yarikoptic-gitmate commented Jun 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jsheunis commented Jun 6, 2023 •

edited by adswa

Loading

codecov Bot commented Jun 7, 2023 •

edited

Loading

jsheunis left a comment •

edited

Loading