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

Improve support for Maven projects and fix build-info URL #618

Merged
merged 2 commits into from
Mar 3, 2022

Conversation

yahavi
Copy link
Member

@yahavi yahavi commented Feb 16, 2022

  • All tests passed. If this feature is not already covered by the tests, I added new tests.

Fix jfrog/jenkins-artifactory-plugin#576
Fix jfrog/jenkins-artifactory-plugin#611

To support using build projects in Maven UI jobs in Jenkins Artifactory plugin, this PR does the following:

  • Add the project in BuildInfoModelPropertyResolver.
  • Fix the created build-info URL when using projects in all cases.
    • If Artifactory's URL and the project key were provided, the createBuildInfoUrl will try to guess the platform URL from Artifactory URL.
    • The URL contained ?project=<project-key> query param instead of ?buildRepo=<project-key>-build-info param.

} else {
url = createBuildInfoUrl(client.getUrl(), build.getName(), build.getNumber(), true);
log.debug("Couldn't create the build-info URL from Artifactory URL: " + client.getUrl());
log.info("Build-info successfully deployed.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we throw an exception in this case? Why are we good with the fact we couldn't create the build-info URL?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. createBuildInfoUrl may fail to resolve the build info URL in the following specific scenario:
The input URL is Artifactory's URL (not platform) + there is a project + the Artifactory URL does not end with /artifactory.
In that case, the build info was published successfully, but it is impossible to guess what is the build-info URL.

buildNumber, timeStamp, project, encode);
}
// If Artifactory's URL doesn't end with "/artifactory", it is impossible to create the URL.
return "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to throw an exception here?

Copy link
Member Author

@yahavi yahavi Mar 3, 2022

Choose a reason for hiding this comment

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

No. This is part of a perfectly legal flow. The build info URL can't be determined in this very specific case. In that case, we may prefer to not display it.
Also, this function is in use in many places and I think we may not like to add a try-catch mechanism for each one of them.

@yahavi yahavi merged commit ef2eda0 into jfrog:master Mar 3, 2022
@yahavi yahavi deleted the maven-proj branch March 3, 2022 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants