Skip to content

feat: introduce fetchFromApi command

Tomas Vik requested to merge 140-reintroduce-make-api-request into main

This MR introduces an interface for making generic API calls. Any client of this interface can defined a request and get it executed.

This MR also changes the code suggestions code for getting an authentication token to take advantage of this new interface.

Original description

We removed the makeApiRequest command from the original authentication MR (feat: support new authentication for code sugge... (!183 - merged)). Thanks to the time-sensitivity of the MR, we disagreed-committed-disagreed. This MR is to discuss returning the makeApiReqeust command.

@pslaughter's comment:


suggestion: So the leaky abstraction from GitLabClient to vscode-mediator-commands has been some technical debt that I've wanted to clean up. For now, the intent has been to keep this as-boring-as-possible so that it's easier to refactor in the near future.

I'm a bit hesitant about adding a COMMAND_MAKE_API_REQUEST command in this MR which is coupled to some urgency. It sounds like COMMAND_MAKE_API_REQUEST improves some interoperability with the GitLab Workflow Extension and the Web IDE, but I'd prefer we handle that problem in a separate MR.

Some concerns about COMMAND_MAKE_API_REQUEST itself

Regarding the COMMAND_MAKE_API_REQUEST specifically, these mediator commands are executable by any other active extension. We're actively wanting to enable the extension marketplace, so having the capability for a wild extension to cause catastrophic user damage with a POST /access_tokens is very undesirable.

Also, the intent of GitLabClient is to encapsulate the GitLab API paths and API types. We lose some nice encapsulation by offloading the parameters of COMMAND_MAKE_API_REQUEST to the client itself.

brainstorm for future iterations:

I think makeApiRequest is something we could actually pass into GitLabClient. This way extensions can reference a nice type safe GitLabClient, which in turn calls makeApiRequest and has different possible implementations based on the environment 🤔


For each thought I'll create a separate thread where we can discuss it 👍

Related to #140 (closed)

Edited by Tomas Vik

Merge request reports

Loading