Better handling of authenitcation across multiple instances

[jamesharris-garmin]

I very often work with AOSP style projects that are composed of repositories across multiple gerrit top level instances that can co-exist in the same workspace.

It seems like I can only really authenticate to a single gerrit instance per workspace under the current forumation of the tool.

Would it be posisble to re-define the tool to better tie into the git API and possibly look for URL authentication via the ~/.netrc authentication or other authentication mechanisms external to vscode settings?

[SanderRonde]

Authentication is currently not the biggest hurdle to multi-instance workspaces imo, the thing I struggle most with is how things would work UI-wise etc. I'm not entirely opposed to the idea (although it does incur quite some complexity) but there are some things where I'm not really sure how things would work. Maybe you (as a multi-repo workspace user) can tell me what you'd expect behavior to be in these situations and maybe it'll turn out to be not super hard to do:

• When you open the changes tab, what would you expect to see? All changes in all instances listed together, a sort of picker where you can pick one, only the ones for the "current" repo, determined by which file is open?
• Similarly what would commands that operate without context do, for example the change picker (in the bottom bar). Would it again show all changes or only the ones for the current repo?

So I think the main question is how would you determine the current repo, or maybe we want all changes from all repos combined together (which would probably make quite some API calls quite a bit slower). If the answer is that we do just take the current file, is it going to be a situation where you are likely to switch repos often? Do you generally stick to a repo or do you hop back and forth?

[Mattadore]

As another multi-instance user, I'd say just having gerrit.gitRepo just be a list of paths to care about would be awesome. I don't personally deal with the multi-auth issue, but supporting multiple gerrit instances under a single workspace would be incredibly helpful.

AOSP has hundreds of gerrit repos within it, so explicitly listing the ones you care about reviews for makes a lot of sense to me. Like, if it's not to expensive to have it just auto-detect the gerrit repo of whatever file you have open and load any relevant review that would be best. "All changes from all repos combined together" does not sound computationally realistic for something like AOSP if you intend to do some recursive search unless you just mean whichever repos are open in the current window.

I am not sure if "load this pre-set list of repos" or "load whatever repos I have open" is best longer term. There are times where I don't even touch a given repo when working, so loading all relevant might actually be more expensive than just loading whatever files I touch. You could also probably filter by the top change of the git repo of a file being authored by the current user before calling into gerrit to load the metadata, to drastically reduce the overhead of checking the repos of all files the user touches. It seems like a lot more work though, just loading from a list gets me like 90% of the functionality I would want if you need to prioritize one.

Do you generally stick to a repo or do you hop back and forth?

I hop around constantly, and generally just expect things to operate on the repo of my currently selected file (all of my custom actions behave like this). I expect that people using this for multi-gerrit in one workspace are probably working on something huge like AOSP.

Would it again show all changes or only the ones for the current repo?

Again, my strong preference is just whatever the repo of the currently selected file is, that's how all my other workflows behave.

maybe we want all changes from all repos combined together

Please no, that sounds like such a headache. Some extensions such as the git and gitlens ones try to do this on some of their windows and it makes those parts totally unusable.

I'm curious if others such @jamesharris-garmin have thoughts on this, but this is my opinion as someone who deals with a massive multi-gerrit workspace regularly. I'm currently just changing which repo the extension points at depending on which review I am doing which is a bit annoying.

[felipecrs]

If possible, I would recommend to use VS Code's Authentication API instead:

https://code.visualstudio.com/api/references/vscode-api#authentication https://www.eliostruyf.com/create-authentication-provider-visual-studio-code/

[SanderRonde]

Alright these are some good answers to my questions :)

I think this does sound like a valid thing to support. To make it work I would like to have such a repository to test it out on. Since I'm not too knowledgeable on this I'll ask you guys. Which (open source) repo is a good example of what you'd like to have supported by this extension? And what does the setup look like? Do you just have a single root folder that you clone all the gerrit instances into? Do you use VSCode's multi-workspace feature? I did briefly look into the AOSP project but it looks like that takes up a massive amount of storage so that's probably not ideal.

[jamesharris-garmin]

I don't know if there is a specific open source project that would cover this use case, but it might be possible to fake it simply by cloning a subproject of AOSP and then getting another simple sub-project like git-review from the openstack guys (opendev.org). These could be in either a multi-root workspace or a single folder with multiple projects (thought the latter case is probably more likely).

It would simply be a case of being able to list witch gerrit servers can log into and then matching the project remote servers against that list of domain names. Then you present a tree view of which projects are associated with which gerrit instance we have configured.

EDIT:

Sorry I am catching up to additional replies. i think most of their feedback is good and agree that when I am working across instances I am bouncing back and forth constantly between projects, so the lowest friction solution to supporting multiple instances would be ideal.

[Mattadore]

I mean, a simple repro is just having a folder with 1-2 multi-level nested subfolders that are Gerrit repos, I don't know if it's much more complex than that. eg: some root workspace, then a/b/somerepo + a/b/someotherrepo + c/d/somethirdrepo. I assume if you get that working it'll probably scale fine, and I can test it on AOSP.

Do you use VSCode's multi-workspace feature?

Different people handle this differently, unfortunately.. there's not a great standard around it. From what I can tell most people do, but I don't, I just have one giant workspace for everything. I assume multi-workspace would just configure this as multiple individual instances, one on each workspace, which I guess would have solved my problem? But I don't really like multi-workspace workflow very much, it's clunky. I'm also not totally sure it solves the problem as I haven't actually tried it.

I did briefly look into the AOSP project but it looks like that takes up a massive amount of storage so that's probably not ideal.

...yeah 😅

[SanderRonde]

Alright it's taken a while (and some 4.5k changed lines of code) but I think I've got it. I've attached a build of what I've got now. It seems to work pretty well from what I can test. See the linked commit's commit message for some more details.

To install the file: rename from a .zip to .vsix (github doesn't allow uploading .vsix files), then go to your editor, to the extensions tab, click the dots on the top-right, click "install from vsix" and pick it there.

vscode--gerrit-1.2.41.zip

[Mattadore]

Nice :smile: really appreciate the support!!!

I will check it out this week and see how it goes.

[jamesharris-garmin]

In trying to configure this I immediately see a problem where I am trying to clone from an ssh:// url and need to specify an alternate http url. (I.e.) the root for the code review URL is https://<server-name>/code-review/ but the repository is cloned from ssh://<server-name>

Should I put the https url in there?

[SanderRonde]

Yes. The HTTP URL is the URL that will be used by the HTTP API. I'm not sure how things are configured in your case, the default setup has both be the same, but I'm pretty sure the URL you use to view your dashboard and such is the one that'll host the API too.

[jamesharris-garmin]

the ssh server is just the domain name .but the gerrit application is hosted form the 'code-review' path.

so the sign in workflow would need to be able to say: oh you have an ssh server, set the root of the gerrit application (hit enter if it is the same)

[jamesharris-garmin]

also if you could have an example layout of how to manually do this in the settings that might be helpful

[SanderRonde]

Ah because it prefills it with the host in the .gitreview file in the "setup credentials" command. Yeah I agree, I'll add that later. Indeed a bit confusing to prefill with possibly the wrong value.

[SanderRonde]

An example:

"gerrit.remotes": {
    "https://<server-name>/code-review/": {
        "username": "...",
        "password": "..."
    }
}

[SanderRonde]

Ah I just realized that I accidentally threw out support for having a different URL from the SSH URL in the current multi-repo approach. Will bring it back again and let you know once I did

[SanderRonde]

Ah turns out it was stil in there but just in another way. I've re-built the extension with better support for it. Now it is a part of the "Enter credentials" command, as well as being part of the gerrit.remotes setting.

vscode--gerrit-1.2.41.zip

[Mattadore]

Activating extension 'SanderRonde.vscode--gerrit' failed: Cannot read properties of undefined (reading 'host'). :(

I would offer more context but cannot figure out what is generating the error, the stacktrace is a mess of vscode internals.

[SanderRonde]

Hmm I can't seem to replicate the error. What are your settings?

[Mattadore]

Is the code assuming that we have gitreview files? It seems to make that assumption, and I have none. Could that be the issue? The credentials code adds a bunch of "host" variables and seems to have a lot of code that depends on git review files existing.

[Mattadore]

    "gerrit.remotes": {
        "default": {
            "url": "https://android review url",
            "username": "someusername",
            "password": "somepass",
        },
    },

same entries as I had before the syntax switch

[SanderRonde]

Ah yeah it is indeed somewhat making the assumption that you have a .gitreview file (and I can reproduce the issue when I remove it myself). Does gerrit still work for you without such a file? And do you know if this is a common setup?

[Mattadore]

Yeah, I have never had a file and everything worked fine before the latest update. The credentials are managed in a different system. I believe this is a fairly common setup, in my experience.

[SanderRonde]

Ah ok. I've changed the setup to be able to catch not having a gitreview file. I think it should work for you then.

vscode--gerrit-1.2.41.zip

[felipecrs]

I think it would be nice to depend as little as possible on git review itself. For example, I don't use it, and it's cumbersome to install.

The feature of downloading changes for example can be easily changed to use only git itself (see the download change links in the Gerrit UI).

[SanderRonde]

Yeah fair point, will try to do that some time soon. Indeed shouldn't be terribly hard.

[Mattadore]

[SanderRonde.vscode--gerrit]Maximum call stack size exceeded
$onExtensionRuntimeError @ mainThreadExtensionService.ts:78

It seems to be stuck in an infinite loop in extension.ts. Something to do with DEV_OVERRIDE?

[SanderRonde]

Oops my bad, indeed looks related to that. Here's another build.

vscode--gerrit-1.2.41 (2).zip

[SanderRonde]

For those interested, I've completely removed the reliance on the git-review tool and, where possible, the .gitreview file. Here's a build of the extension with those changes. (CC @felipecrs)

vscode--gerrit-1.2.41-2.zip

[felipecrs]

That's really nice!

[felipecrs]

@SanderRonde, just wondering, are these changes part of the latest release already?

[Mattadore]

I will check it out soon, I have been a bit busy, but seems very promising :D

[SanderRonde]

I've not released either of these changes yet. I'm kind of hoping you guys can test out the multi-repo variant since I can't test it in my day-to-day workflow. After that one has gone live the git-review change will probably also go live soon after since I was able to test that myself.

[felipecrs]

Got it. I don't personally use multi-repo feature, I think.

But I will try out the vsix and let you know. I'm specially interested on the git-review changes.

[Mattadore]

Tried to run "git config --get remote.origin.url", but failed
Stdout: 
Stderr: 
Error: Command failed: git config --get remote.origin.url

soooooooooo I don't use "origin" like, I could add "origin" but it would need to be done manually for each repo. I don't think it is unreasonable to assume "gerrit" remote exists, but better would probably be to just have a setting for default remote name. Ideally once I get this working, I could export my config to other Android developers and have it "just work" ™️

I know my workflow is weird, but there is an extremely large population of developers who have a similar weird gerrit workflow and could benefit from this

Additionally, one time adding the remote gave: 'SanderRonde.vscode--gerrit' failed: Cannot read properties of undefined (reading 'body'). ??? but I haven't seen that error again since o.O

[felipecrs]

I don't use "gerrit" in my git remotes, I only use "origin", because I only have one remote, which is gerrit itself.

Perhaps trying first "gerrit" then "origin" would be a good default?

I think it would cover 99% of the cases, and later perhaps if someone complains, add the option to configure it.

[Mattadore]

FWIW almost none of my repos use "gerrit" or "origin" and that would immediately break, I actually retract my prior statement about being able to assume "gerrit" exists. Also, it is trying to access data from the remote itself which immediately fails with "UnsupportedProtocolError" because we don't use "ssh:" or "https:" but a weird non-standard prefix. The thing is, all of this worked before the multi-repo split, and still works on the current public version, so I am wondering what I can do to just make it behave like it did before. It seems like this added more assumptions, which are.. maybe helpful? but don't exist on the single-repo version.

[felipecrs]

I guess the key is to keep reading information from .gitreview if such file exists.

[Mattadore]

Actually, it seems like this is maybe just a.. bug? It is trying to figure out what the gerrit server is in order to access "config/server/version" , but it's moot because I already passed the server URL to the config so it should just use that, I think it doesn't need to know what my remote is or any of that.

"[prefix][repo]/config/server/version" is what it's trying to access but that isn't even a thing, what it wants is "[url]/config/server/version"

[SanderRonde]

Will look at this in detail more this weekend. But I'm pretty sure the git-related issues (origin etc) are due to me removing the reliance in git-review. I've made a few assumptions there that I thought were correct but apparently not all were.

You can still find a multi-repo build without these changes over here: https://github.com/SanderRonde/VSCode-Gerrit/issues/70#issuecomment-2278320556

[Mattadore]

It's no rush, I appreciate all the support :)))

[SanderRonde]

Some changes to the git-review related changes in this new version. I've made it go down a list of remotes, falling back to the first remote in git remote when there is no match.

You should be able to set the correct API URL by setting the url field of the new gerrit.remotes field. You'll probably want to use default as a key for the setting (at least while testing).

vscode--gerrit-1.2.41.zip

[Mattadore]

It is still using the git remote as the host for review, not the "url" param.

I am actually not sure where the "url" config param is used at all in the code, but am maybe just blind?

[SanderRonde]

Yes I think that makes sense or am I missing something? The git remote is the one that git-review changes are git-pushed to. The URL is used for the HTTP API, which is where the extension gets most of its data from.

[Mattadore]

It appears to be trying to make https requests to "https://" + remote + repo "/config/server/version" which will not work, and it was not doing previously. I cannot share the exact string, but it's not a valid http request and the remote does not even have a TLD, we use a weird auth provider. I am wondering if it is maybe just accidentally using the remote instead of the URL host param passed to the config for this; this all still works just fine on the main branch.

[SanderRonde]

I did change quite a bit about the structure of the API in order to make multi-instance usage work. So do expect some things to be broken and to not work, mostly because it's quite hard for me to test this against a real-life use case.

I think I did fix the bug you're mentioning in the package below, I'm not 100% sure though (since I can't really verify). Could you check for me whether this helps? If not, a few things you can do:

• Use the "Setup credentials" command to set it up from scratch, that might help
• Check the devtools and its logs. There'll be a bunch of Dev: ... logs there. It would be a great help if you could share those (of course feel free to censor them)

vscode--gerrit-1.2.41-3.zip

[Mattadore]

It works! Kind of. It successfully loads all the ongoing reviews under a given repo, and if I change repos it then loads those. However, comments don't work at all and I have no idea why, the devtools aren't returning any issues nor is the extension output. Importantly it's not just per-file comments, even opening an ongoing review does not load comments, they just don't seem to work at all.

It is also bit slow, though I think there are some possible fixes, as it seems to be an issue of just "a ton of requests at once". I think batching requests into one mega-request might be possible, as the webui seems to do this, and we can also probably just lower the amount of changes it fetches per-category at once if that doesn't work.

[Mattadore]

I did not mess with the auth config at all, the one I was trying previously immediately worked on the new version.

[SanderRonde]

I didn't get to the comments issue yet but I did manage to speed up the requests a lot. While it wasn't a matter of syncing up requests (there's only one request that supports multiple changes), it was a matter of removing unnecessary requests by rewriting some code. Loading the changes panel and expanding changes is now much faster :)

Thanks for the suggestion btw, I also released this optimization to the main extension branch

vscode--gerrit-1.2.41-4.zip

[SanderRonde]

I tried to replicate the no-comments issues but they do work for me. Some more questions:

• Are you able to place any comments? What happens to them?
• Do you see comment icons next to files in the changes panel on the left?
• Are you sure they're not resolved and that's why they're initially collapsed?

After these questions the best thing to do is probably for me to make a debug build for you that logs to the console which comments are loaded from the API and to figure out why they're not displayed.

[Mattadore]

I got it to load once, by like.. re-loading it a couple of times and going through the change menu directly. The comment icons do not normally show up, and the gutter to add comments isn't there unless I go through the change menu directly, but it seems to be able to load comments, it just isn't.

I am starting to suspect this is related to call volume, as the client seem to be sending a lot of duplicate calls and the server is definitely rate limiting them and failing some of the transactions. I think the calls to get comments might just be getting dropped by the server rate limiting?