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: collect and extract jenkins jobs from multibranch pipelines #7213

Merged
merged 3 commits into from
Apr 18, 2024

Conversation

gustavobini
Copy link
Contributor

Summary

Collects and extracts job, build, and stages data from multi-branch Jenkins pipelines.

Does this close any open issues?

#5633

@gustavobini gustavobini marked this pull request as ready for review March 25, 2024 14:49
@d4x1 d4x1 requested a review from klesh March 26, 2024 02:53
@klesh
Copy link
Contributor

klesh commented Apr 1, 2024

Hi, @gustavobini , Sorry, I have been working on some performance optimization recently, may I come back to you maybe next week?

@gustavobini
Copy link
Contributor Author

Hi, @gustavobini , Sorry, I have been working on some performance optimization recently, may I come back to you maybe next week?

yes that's perfectly fine!

backend/plugins/jenkins/api/remote_api.go Show resolved Hide resolved
ConnectionId: connection.ID,
FullName: jenkinsJob.FullName,
ConnectionId: connection.ID,
ConnectionEndpoint: connection.Endpoint,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we store the extra information in the _tool_jenkins_jobs table instead of passing it around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The class is used at the tasks as the input between multi-branch and other jobs, so it needs to be available before. The ConnectionEndpoint cannot be stored in the _tool_jenkins_jobs db, as that will break the plugin in case the connection endpoint changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see.
How about that we load the information during the PrepareTaskData phase?
By doing so, we could rerun the plan even after the connection endpoint changed without the need to update every related blueprints, especially for recurring Advanced Blueprint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get what you mean. I'll do that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@klesh I've moved the properties. I'm not happy with how it is, but I think it's the simplest. All the job names and path validations and assignments being done around make me hesitant to do more. I actually wanna get that refactored at some point if possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the update.
Would you like me to merge it now so you may take time for the refactor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@klesh yes please! and thank you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't tell what's wrong with the lint-commit-message error 🤔

klesh
klesh previously approved these changes Apr 12, 2024
Copy link
Contributor

@klesh klesh left a comment

Choose a reason for hiding this comment

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

LGTM

@klesh
Copy link
Contributor

klesh commented Apr 16, 2024

lint commit message checks if all commit messages of the PR fit https://www.conventionalcommits.org/en/v1.0.0/, it is optional.
Please resolve the conflict and I will merge it ASAP

@gustavobini
Copy link
Contributor Author

lint commit message checks if all commit messages of the PR fit https://www.conventionalcommits.org/en/v1.0.0/, it is optional. Please resolve the conflict and I will merge it ASAP

@klesh done!

@gustavobini gustavobini requested a review from klesh April 16, 2024 10:11
@klesh klesh merged commit 6dca288 into apache:main Apr 18, 2024
10 checks passed
@gustavobini gustavobini deleted the jenkins-multibranch branch April 18, 2024 07:55
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

Successfully merging this pull request may close these issues.

None yet

2 participants