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

feat(azuredevops): support disabled Repos #7657

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mr-ks
Copy link
Contributor

@mr-ks mr-ks commented Jun 23, 2024

Summary

It is now possible to select disabled repositories as a remote scope. However, disabled repositories are not included in tasks related to domain code and code review, as the Azure DevOps API returns a 4xx status code for the required API calls.

A new column called is_disabled has been added to the _tool_azuredevops_go_repos table to support disabled repositories. For existing entries, the default value for this column is set to false.

Does this close any open issues?

Closes #7504

mr-ks added 3 commits June 9, 2024 19:46
It is now possible to select disabled repositories as a remote scope. However, disabled repositories are not included in tasks related to domain `code` and `code review`, as the Azure DevOps API returns a 4xx status code for the required API calls.

A new column called `is_disabled` has been added to the `_tool_azuredevops_go_repos` table to support disabled repositories. For existing entries, the default value for this column is set to `false`.
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. component/plugins This issue or PR relates to plugins pr-type/feature-development This PR is to develop a new feature labels Jun 23, 2024
@@ -39,7 +39,7 @@ var CollectApiPullRequestCommitsMeta = plugin.SubTaskMeta{
EntryPoint: CollectApiPullRequestCommits,
EnabledByDefault: true,
Description: "Collect PullRequestCommits data from Azure DevOps API.",
DomainTypes: []string{plugin.DOMAIN_TYPE_CROSS, plugin.DOMAIN_TYPE_CODE_REVIEW},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @klesh,
what should be the expected outcome if the user creates a scope configuration with only Cross Domain as the data entity? Should this also include the tasks of domain Code Review and Source Code Management?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Code review" should be included alongside "Cross Domain".
"Cross Domain" means it would produce "accounts" records as a byproduct.
"Source Code Management" should not be included because it represents the git repo code base itself.
"Domain Types" are meant to select which subtasks to run but not limit what they could produce. It is simpler this way and most likey more desirable for users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying. If we classify both the collect-,convert-,extractAccounts tasks and the pullRequests tasks as Cross Domain, we will face issues selecting the correct tasks for disabled ADO repos (and GH repos). Collecting Accounts will work fine, but collecting pull requests will fail. What are your thoughts on this?

Maybe something like this?

entities := scopeConfig.Entities without Cross Domain

tasks := helper.MakePipelinePlanSubtasks(subtaskMetas, entities)
tasks = append(tasks, collect-,convert-,extractAccounts)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mr-ks Good question, I think we could use the IgnoreAndContinue technique for certain HTTP status codes as a workaround.

if azuredevopsRepo.Type != models.RepositoryTypeADO {
repo, scopeConfig := scope.Scope, scope.ScopeConfig

if len(scopeConfig.Entities) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we are treating this condition as Selected All because it has no point to collect a scope without any entity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hint, fixed.

var err errors.Error
repo, scopeConfig := scope.Scope, scope.ScopeConfig

if len(scopeConfig.Entities) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scopeConfig.Entities are considered as "All Entities".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hint, fixed.

}

func (*disabledRepos) Version() uint64 {
return 20240806100000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the numbers from the file name and the version are different and both incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed and updated the date to today.

@@ -39,7 +39,7 @@ var CollectApiPullRequestCommitsMeta = plugin.SubTaskMeta{
EntryPoint: CollectApiPullRequestCommits,
EnabledByDefault: true,
Description: "Collect PullRequestCommits data from Azure DevOps API.",
DomainTypes: []string{plugin.DOMAIN_TYPE_CROSS, plugin.DOMAIN_TYPE_CODE_REVIEW},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Code review" should be included alongside "Cross Domain".
"Cross Domain" means it would produce "accounts" records as a byproduct.
"Source Code Management" should not be included because it represents the git repo code base itself.
"Domain Types" are meant to select which subtasks to run but not limit what they could produce. It is simpler this way and most likey more desirable for users.

@mr-ks mr-ks changed the title Support Disabled Azure DevOpsRepos feat(azuredevops): support disabled Repos Jun 24, 2024
Empty entities are not treated as 'Selected All' since collecting a scope without any entity is pointless
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/plugins This issue or PR relates to plugins pr-type/feature-development This PR is to develop a new feature size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug][Blueprints] 400 error when adding Azure DevOps scope to existing project
2 participants