Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ado2gh] RewirePipeline - Accept definitionId (int) for ado-pipeline parameter #1222

Open
tiago-soczek opened this issue Feb 7, 2024 · 3 comments

Comments

@tiago-soczek
Copy link
Contributor

Description

Currently, we provide the name or path of the pipeline definition (ado-pipeline) to perform the rewire.

With this value, we look for the definitionId (int) - this is expensive as it is necessary to enumerate all Definitions as the API does not provide a method for this.

    // removed code for brevity
    public virtual async Task<int> GetPipelineId(string org, string teamProject, string pipeline)
    {
        // Check if we have the pipeline id cached - this will gone after process kill / execution
        if (_pipelineIds.TryGetValue((org.ToUpper(), teamProject.ToUpper(), pipelinePath.ToUpper()), out var result))
        {
            return result;
        }

        var url = $"{_adoBaseUrl}/{org.EscapeDataString()}/{teamProject.EscapeDataString()}/_apis/build/definitions?queryOrder=definitionNameAscending";
        
        // expensive call
        var response = await _client.GetWithPagingAsync(url);

To avoid this, the user could have the option to directly pass definitionId - this would avoid unnecessary calls and make the process faster.

The suggestion is to use the same parameter (ado-pipeline) and in the implementation, consider that it is the definitionId if it is a number, otherwise, follow the current process - convenient for those who do not have the definitionId at hand.

    // removed some code for brevity
    public virtual async Task<int> GetPipelineId(string org, string teamProject, string pipeline)
    {
        // Check if the pipeline is a number - the id itself
        if (int.ParseInt(pipeline, out var definitionId)) {
            return definitionId;
        }

        /// continue with the rest of the code

It makes sense? If so, I will provide a PR.

@brianaj
Copy link
Collaborator

brianaj commented Feb 7, 2024

@tiago-soczek I would suggest creating a new option --ado-pipeline-id instead seeing as we have a very specific use for --ado-pipeline to do the look up of the id.

@tiago-soczek
Copy link
Contributor Author

It also works, using two options it will be necessary to change the validation (--ado-pipeline OR --ado-pipeline-id is mandatory), but I think this is not a problem.

@brianaj
Copy link
Collaborator

brianaj commented Feb 7, 2024

Yeah that validation would live here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants