NF: make extra_remote_settings configurable in git config

[matrss]

This PR makes the extra_remote_settings attribute for GitHub-like create_sibling targets configurable on a per target basis. One usecase would be to override an annex-ignore setting, which is otherwise unconditionally set. E.g. if we host a gitea server which does support git-annex at example.com, we could set:

[datalad "extra_remote_settings.example.com"]
	annex-ignore = false

in e.g. ~/.gitconfig to override the default of not using git-annex with gitea.

This approach does have the issue that the settings are apparently set before first enabling the remote. This leads to spurious warnings like this when creating the sibling:

[INFO   ] Could not enable annex remote origin. This is expected if origin is a pure Git remote, or happens if it is not accessible. 
[WARNING] Could not detect whether origin carries an annex. If origin is a pure Git remote, this is expected.

but a subsequent push does work with annex enabled.
The code for create_sibling_gin apparently sets annex-ignore afterwards, but I am not sure if there are any implications for other possible options in extra_remote_settings which might need to be set before or after first enabling the remote. Feedback welcome.

Fixes #7179.

[codecov]

Codecov Report

Base: 88.07% // Head: 90.68% // Increases project coverage by +2.60% 🎉

Coverage data is based on head (2b4a079) compared to base (34a28a6).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7213      +/-   ##
==========================================
+ Coverage   88.07%   90.68%   +2.60%     
==========================================
  Files         324      324              
  Lines       43915    43922       +7     
  Branches     5842     5839       -3     
==========================================
+ Hits        38680    39832    +1152     
+ Misses       5220     4075    -1145     
  Partials       15       15              

Impacted Files	Coverage Δ
datalad/distributed/create_sibling_gin.py	83.33% <ø> (ø)
datalad/distributed/create_sibling_gitea.py	59.45% <ø> (ø)
datalad/distributed/create_sibling_github.py	52.23% <ø> (ø)
datalad/distributed/create_sibling_gitlab.py	68.90% <ø> (ø)
datalad/distributed/create_sibling_gogs.py	100.00% <ø> (ø)
datalad/distributed/create_sibling_ghlike.py	65.96% <100.00%> (+1.29%)	⬆️
datalad/local/remove.py	95.31% <0.00%> (-1.57%)	⬇️
datalad/interface/base.py	95.05% <0.00%> (-0.36%)	⬇️
datalad/core/local/run.py	95.83% <0.00%> (-0.33%)	⬇️
datalad/config.py	97.27% <0.00%> (-0.23%)	⬇️
... and 51 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

[bpoldrack]

Great, you are attacking this, @matrss !

In principle looks about right, will have a closer look later.

One remark already: I think it's preferable to have a config name that refers to create-sibling-ghlike or something along those lines, b/c it's not going to be respected by anything else. So, it's a customization of the command behavior rather than something that applies to any matching remote. However, that can obviously be addressed last.

[bpoldrack]

So, I finally got back to this, @matrss.

Apart from the earlier comment about the config name, there are two more remarks from my POV:

1. When introducing a new config, please add it to interface/common_cfg.py:_definitions. Existing entries should tell you what to put in there, if not feel free to ask or to leave it to me to add this. (This is where for example datalad configuration would look it up)

2. I am wondering, whether the target specification via urlparse().netloc is good enough. (as opposed to being able to specify a specific URL to match) I suppose in reality it's OK.

WDYT, @datalad/developers (also about naming the config)?

[matrss]

Thanks for the feedback!

Regarding 1: I took some inspiration from credential handling in regard to how I select the config section with netloc. The datalad.credential.* section is also not listed in interface/common_cfg.py:_definitions and it is not obvious to me how an entry there should look like for a configuration section which essentially contains a wildcard part.

Regarding 2: Since this is meant for overriding the options per distinct target I think matching by netloc should be enough, since every target host should be uniquely identifiable by an <ip/domain>:<port> combo. In contrast to the credential handling I do not see a use case for multiple different settings per target.

I am still not sure if this general approach of overriding the extra_remote_settings with any provided option is the best way to go though. Since it is happening so early it produces the above mentioned misleading Could not enable annex remote warning and it could be used to set any option, no matter how much sense that actually makes. Maybe this should instead be a single configuration option to set annex-ignore=false after creating the repos, in a similar fashion to how create_sibling_gin does it.

[bpoldrack]

it is not obvious to me how an entry there should look like for a configuration section which essentially contains a wildcard part.

That's a good point. I failed to pay attention to this aspect. Indeed such configs are documented on the docstrings of the commands using them (which will be a bit tricky here - need to check myself).

In contrast to the credential handling I do not see a use case for multiple different settings per target.
I am still not sure if this general approach of overriding the extra_remote_settings with any provided option is the best way to go though.

I actually like that it's more general than just addressing the one usecase we are aware of. If users explicitly demand a config to be set, they will know why and probably better than us. I like to not be too paternalistic in that regard, just b/c we didn't think it could make sense in some setup. Plus: This way there's no need to actually release a new version for every single case where this may come in handy.

Since it is happening so early it produces the above mentioned misleading Could not enable annex remote warning

I am not sure about this. As far as I can see it's not exactly happening too early in the command execution, but possibly too early in the workflow. IIRC, GIN would behave that way, too: If the repo is fresh on their end and doesn't have any annex yet, it behaves as if it doesn't support annex. Don't know whether that holds for the gitea fork you are using.
What does git annex enableremote origin say right after creation w/o having pushed anything yet?

[matrss]

What does git annex enableremote origin say right after creation w/o having pushed anything yet?

$ git annex enableremote origin
enableremote origin failed
enableremote: 1 failed

So that does fail too.

I guess for GIN what's happening is that the repo and remote are created, and then annex-ignore is set to false at the end while with this override here, annex-ignore is set to false and then the repo and remote are created. For gin there is no warning, since you usually do not enableremote by itself, but instead just push to the remote.

[bpoldrack]

while with this override here, annex-ignore is set to false and then the repo and remote are created.

I may miss something, but I don't think so. You enhance that dict early on, yes. But it's only used to set the config at the end of create_repos via an internal call to datalad siblings. And this is trying to enable. But the remote repo should already exist at this point. Hence, it's the same as calling git annex enableremote after create-sibling-*. Which is why I asked.

So that does fail too.

I see. But that means, no matter how this would be implemented, as long as it's part of create-sibling-* it will produce this warning. (Well, not exactly - we can set the config w/o trying it out, but that, too, is a bit quirky; Subsequent commands can then produce this warning just as misleadingly)
I suppose, enabling only stops failing after the remote repo actually has a git-annex branch?

[matrss]

But it's only used to set the config at the end of create_repos via an internal call to datalad siblings. And this is trying to enable. But the remote repo should already exist at this point. Hence, it's the same as calling git annex enableremote after create-sibling-*.

Ah yes, I think you are right. That makes sense.

we can set the config w/o trying it out

I think that is exactly what create_sibling_gin does then.

I suppose, enabling only stops failing after the remote repo actually has a git-annex branch?

Not sure about the specifics of how git-annex determines support, but it certainly does stop failing when a git-annex branch exists on the remote. TBH I think this is a bug in how git-annex determines support for a remote, which then leads to the warning here. There should be some git-annex support "discovery mechanism" that also works for freshly created empty repositories. Although enableremote would probably still fail, since the remote can not actually be used yet if it is empty.

[bpoldrack]

There should be some git-annex support "discovery mechanism" that also works for freshly created empty repositories. Although enableremote would probably still fail, since the remote can not actually be used yet if it is empty.

Now that you say it, and I played a bit, I think you actually just pointed out a bug in datalad siblings. Need to double-check my current impression is correct, but: Normally annex would report that it cannot use a remote if it's not supported and set annex-ignore back to true. But this does not seem to be what's happening. It only fails to enable but doesn't claim a lack of support and leaves annex-ignore untouched (just tried with an empty local repo).

If that holds true, then the bug is in datalad siblings as it interprets any failure of enableremote as the same notion of unusable. We need to detect Remote origin not usable by git-annex; setting annex-ignore instead and not issue a misleading warning.

[matrss]

Yes, enableremote seems to behave differently for different targets in this case. While the above fail message is generated for the gitea instance with git-annex support and the annex-ignore option is left alone, a gitlab remote produces some more output:

$ git annex enableremote jugit

  Unable to parse git config from jugit

  Remote jugit does not have git-annex installed; setting annex-ignore

  This could be a problem with the git-annex installation on the remote. Please make sure that git-annex-shell is available in PATH when you ssh into the remote. Once you have fixed the git-annex installation, run: git annex enableremote jugit
enableremote jugit 
  Unable to parse git config from jugit

  Remote jugit does not have git-annex installed; setting annex-ignore

  This could be a problem with the git-annex installation on the remote. Please make sure that git-annex-shell is available in PATH when you ssh into the remote. Once you have fixed the git-annex installation, run: git annex enableremote jugit
failed
enableremote: 1 failed

and then sets annex-ignore=true for the remote.

[bpoldrack]

Thank you, for the confirmation, @matrss!

Then let's ignore the warning issue for this PR - I'll fix it separately.

I am checking how best to inject the config into the docs for the commands. Little different here than with other commands, because of the way things are subclassed here.

One more thing on the config name: We'd need to use - instead of _. That's because we make the configs also available via env var. The way this works is: Turn the env var to lower case, replace _ with . and __ with -. Then it's an override of the resulting config name. Even if that may be ugly with the URL part, let's be consistent with that pattern.

[matrss]

Then let's ignore the warning issue for this PR - I'll fix it separately.

Great 👍

One more thing on the config name: We'd need to use - instead of _. That's because we make the configs also available via env var. The way this works is: Turn the env var to lower case, replace _ with . and __ with -. Then it's an override of the resulting config name.

Alright, I will update that as soon as we have settled on a full name for this configuration. I guess datalad.create-sibling-ghlike.extra-remote-settings.<netloc>.* would be the most straightforward option, but maybe there is something better.

[bpoldrack]

Re documentation:

Sadly, we do not have a nice way of injecting documentation for this config from a central place into all the command docs that it would apply to (because we only account for a command-specific doc block and dynamic, parameter-specific blocks) . For now, the cleanest way seems to be to "manually" put it in all of them. Here is a suggestion, that gives you all the anchor points. Feel free to rephrase:

Diff:

diff --git a/datalad/distributed/create_sibling_gin.py b/datalad/distributed/create_sibling_gin.py
index e0db12b8e..fa318dc32 100644
--- a/datalad/distributed/create_sibling_gin.py
+++ b/datalad/distributed/create_sibling_gin.py
@@ -71,6 +71,19 @@ class CreateSiblingGin(Interface):
     generated on the platform (Account->Your Settings->Applications->Generate
     New Token).
 
+    This command can be configured with
+    "datalad.create-sibling-ghlike.extra-remote-settings.NETLOC.KEY=VALUE" in
+    order to add any local KEY = VALUE configuration to the created sibling in
+    the local `.git/config` file. NETLOC would specify the Gin domain to apply
+    this configuration to.
+    This leads to a behavior that is equivalent to calling datalad's
+    ``siblings('configure', ...)``||``siblings configure`` command with the
+    respective KEY-VALUE pair after creating the sibling.
+    The configuration, like any other, could be set at user- or system level, so
+    users do not need to add this configuration to every sibling created with
+    the service at NETLOC themselves.
+
     .. versionadded:: 0.16
     """
 
diff --git a/datalad/distributed/create_sibling_gitea.py b/datalad/distributed/create_sibling_gitea.py
index c644bfa28..cee4d18f7 100644
--- a/datalad/distributed/create_sibling_gitea.py
+++ b/datalad/distributed/create_sibling_gitea.py
@@ -94,6 +94,20 @@ class CreateSiblingGitea(Interface):
     In order to be able to use this command, a personal access token has to be
     generated on the platform (Account->Settings->Applications->Generate Token).
 
+    *Configuration*
+
+    This command can be configured with
+    "datalad.create-sibling-ghlike.extra-remote-settings.NETLOC.KEY=VALUE" in
+    order to add any local KEY = VALUE configuration to the created sibling in
+    the local `.git/config` file. NETLOC would specify the Gitea domain to apply
+    this configuration to.
+    This leads to a behavior that is equivalent to calling datalad's
+    ``siblings('configure', ...)``||``siblings configure`` command with the
+    respective KEY-VALUE pair after creating the sibling.
+    The configuration, like any other, could be set at user- or system level, so
+    users do not need to add this configuration to every sibling created with
+    the service at NETLOC themselves.
+
     .. versionadded:: 0.16
     """
 
diff --git a/datalad/distributed/create_sibling_github.py b/datalad/distributed/create_sibling_github.py
index fe176b1f7..a039a4b88 100644
--- a/datalad/distributed/create_sibling_github.py
+++ b/datalad/distributed/create_sibling_github.py
@@ -129,6 +129,18 @@ class CreateSiblingGithub(Interface):
     generated on the platform (Account->Settings->Developer Settings->Personal
     access tokens->Generate new token).
 
+    This command can be configured with
+    "datalad.create-sibling-ghlike.extra-remote-settings.NETLOC.KEY=VALUE" in
+    order to add any local KEY = VALUE configuration to the created sibling in
+    the local `.git/config` file. NETLOC would specify the Github domain to
+    apply this configuration to.
+    This leads to a behavior that is equivalent to calling datalad's
+    ``siblings('configure', ...)``||``siblings configure`` command with the
+    respective KEY-VALUE pair after creating the sibling.
+    The configuration, like any other, could be set at user- or system level, so
+    users do not need to add this configuration to every sibling created with
+    the service at NETLOC themselves.
+
     .. versionchanged:: 0.16
        || REFLOW >>
        The API has been aligned with the some
diff --git a/datalad/distributed/create_sibling_gitlab.py b/datalad/distributed/create_sibling_gitlab.py
index 4845748ed..6bd89695f 100644
--- a/datalad/distributed/create_sibling_gitlab.py
+++ b/datalad/distributed/create_sibling_gitlab.py
@@ -119,6 +119,19 @@ class CreateSiblingGitlab(Interface):
         Project location/path used for a datasets at GitLab instance
         SITENAME (see --project). Configuring this is useful for deriving
         project paths for subdatasets, relative to superdataset.
+
+    In addition, this command can be configured with
+    "datalad.create-sibling-ghlike.extra-remote-settings.NETLOC.KEY=VALUE" in
+    order to add any local KEY = VALUE configuration to the created sibling in
+    the local `.git/config` file. NETLOC would specify the Gitlab domain to
+    apply this configuration to.
+    This leads to a behavior that is equivalent to calling datalad's
+    ``siblings('configure', ...)``||``siblings configure`` command with the
+    respective KEY-VALUE pair after creating the sibling.
+    The configuration, like any other, could be set at user- or system level, so
+    users do not need to add this configuration to every sibling created with
+    the service at NETLOC themselves.
+
     """
     _params_ = dict(
         path=Parameter(
diff --git a/datalad/distributed/create_sibling_gogs.py b/datalad/distributed/create_sibling_gogs.py
index a5da51a03..4f2920900 100644
--- a/datalad/distributed/create_sibling_gogs.py
+++ b/datalad/distributed/create_sibling_gogs.py
@@ -63,6 +63,18 @@ class CreateSiblingGogs(Interface):
     generated on the platform
     (Account->Your Settings->Applications->Generate New Token).
 
+    This command can be configured with
+    "datalad.create-sibling-ghlike.extra-remote-settings.NETLOC.KEY=VALUE" in
+    order to add any local KEY = VALUE configuration to the created sibling in
+    the local `.git/config` file. NETLOC would specify the Gogs domain to apply
+    this configuration to.
+    This leads to a behavior that is equivalent to calling datalad's
+    ``siblings('configure', ...)``||``siblings configure`` command with the
+    respective KEY-VALUE pair after creating the sibling.
+    The configuration, like any other, could be set at user- or system level, so
+    users do not need to add this configuration to every sibling created with
+    the service at NETLOC themselves.
+
     .. versionadded:: 0.16
     """

Re config name:

I guess datalad.create-sibling-ghlike.extra-remote-settings..* would be the most straightforward option

Agree, it's straightfoward. A bit monstrous, but not using an ambiguous or misleading namespace. We could make it a bit shorter: remote-extras or remote-settings, I suppose. There are no other "remote-settings", and we don't really need to match the dict's name in a user-facing config name.

[matrss]

I updated the code to use "datalad.create-sibling-ghlike.extra-remote-settings..*" for the configuration. I think it should be extra-remote-settings instead of just remote-settings. The name remote-settings implies an exhaustive list of all the configuration items set, atleast in my head. Since we override the default, what we actually do is specify extra configuration items on top of that. Therefore extra-remote-settings.

I have added your suggested documentation snippet with a slight modification at the suggested places. I also made the commit message a bit more verbose.

[bpoldrack]

Great, @matrss! Thank you for discussing the issue (and for the PR itself, of course)!

You should be added to CONTRIBUTORS automatically soon after I merge this. Feel free to add yourself to .zenodo.json for citation!

I'd add a small test for this, but unless you want that exercise I can do that myself.

[qlty-cloud-legacy]

Code Climate has analyzed commit 2b4a079 and detected 0 issues on this pull request.

View more on Code Climate.

[matrss]

Added myself to the zenodo file.

Feel free to add a test for this.

Thank you too for the great experience with this Issue and PR!

[yarikoptic-gitmate]

PR released in 0.18.0