jfrog / teamcity-artifactory-plugin

TeamCity plugin that enables traceable build artifacts with Artifactory
https://www.jfrog.com/confluence/display/JFROG/TeamCity+Artifactory+Plug-in
Apache License 2.0
42 stars 47 forks source link

Build info link does not work on TeamCity 2018 #43

Open pavel-spacil opened 6 years ago

pavel-spacil commented 6 years ago

Hello, we have just upgraded staging env to TeamCity 2018.1 EAP2 (Art plugin in version 2.6.0) and Artifactory 6.0.1 and we are facing an issue that build info is not accessible after upload to Artifactory. TeamCity changed separator in full project name from double colon '::' to slash '/' and thus link from TeamCity result page doesn't work and shows empty build page, same happens when you go from Builds page. This issue is related to issue #30 which should be increased on priority.

TCAgentUser commented 6 years ago

it is also breaking download via filespec functionality.

eyalbe4 commented 6 years ago

@pavel-spacil and @TCAgentUser, We'll look into this soon and update here. Thanks for letting us know!

TCAgentUser commented 6 years ago

I think we will fork out and provide pull request today if possible as we are having production stop from this

eyalbe4 commented 6 years ago

@TCAgentUser, Your help is appreciated! We're also working now on reproducing and fixing the issue. We'll send out another update soon.

DimaNevelev commented 6 years ago

@TCAgentUser , The build info link is indeed broken. However, I didn't manage to reproduce the download via filespec issue. I executed a simple "Command Line" build. I used "TeamCity Professional 2018.1 (build 58245)". My download filespec is very simple: "{ "files": [ { "pattern": "tests/*", "target": "Bazinga/" } ] }". In order to help us reproduce the download issue can you please provide us with additional information such as your build log, exceptions (if there are any), your download filespec and any additional information you think that can help us.

TCAgentUser commented 6 years ago

@DimaNevelev we are already modifying the sources in our own fork. We have updated to latest teamcity api 2018.1 and replacing getFullName with getBuildTypeExternalId. The latter is the more unique part for a build configuration (it can change, but if it does a new functionality to the plugin must handle already finished builds to update the build.name property, this is also true for the getFullName)

For more detailed error description please visit https://youtrack.jetbrains.com/issue/TW-56014

Specifically for the build url link, we are going to do a check on the teamcityserver version, such that only builds going forward are update to use externalid, such that already finished builds are not broken.

jetersen commented 6 years ago

@DimaNevelev issue with our file spec is the following, we are using the build property

{
  "files": [
   {
      "pattern": "%system.ArtifactoryPattern%",
      "target": "%system.ArtifactoryTarget%",
      "build": "%system.ArtifactoryBuild%",
      "flat": "true",
      "explode": "true"
   }
  ]
}

ArtifactoryBuild used to be Project:: Tests :: Create package/%dep.Project_Main_Tests_CreatePackage.build.counter%

But with our current changes to this plugin can be. %dep.Project_Main_Tests_CreatePackage.system.teamcity.buildType.id%/%dep.Project_Main_Tests_CreatePackage.build.counter%

Since we are using build.getBuildTypeExternalId() instead of build.getFullName().

eyalbe4 commented 6 years ago

@casz, As I understand it, this is an example for the old build name: TRIOS :: Main :: SpecFlow Tests :: Create Package

and this is the same build name following the changes in TeamCity: TRIOS/Main/SpecFlow Tests/Create Package

But what will be the build name following the suggested changes to the Artifactory Plugin code?

jetersen commented 6 years ago

@eyalbe4 the name would be Main_Trios_SpecFlowTests_CreatePackage because we try to avoid renaming our ids

We are using this approach to fix old build names http://artifactorydk:8081/artifactory/api/build/rename/TRIOS %20::%20Main%20::%20SpecFlow%20Tests%20::%20Create%20package?to=Main_Trios_SpecFlowTests_CreatePackage Going to create a script using the REST api and share it for anyone one affecting 👍 To fix URLs for older builds on Artifactory

pavel-spacil commented 6 years ago

I've forked this plugin and implemented the custom build name provider which uses single colon without spaces as name separator, it looks better than build ID with underscores from my PoV. Also I've added logic to store full link to custom data storage for each build to not loose connection when build is renamed and to not introduce any regression after upgrade of plugin/TC. Please see commit https://github.com/pavel-spacil/teamcity-artifactory-plugin/commit/3c184b344184b11dd41d752f09720c9fac57e791

jetersen commented 6 years ago

@pavel-spacil does this fix it for older as well as new builds?

jetersen commented 6 years ago

The problem as I see it... is that getFullName() still does not solve it, yes you can patch it and fix, but I still think the actual fix is to use getBuildTypeExternalId().

This way we can actually retrieve that parameter in TeamCity. Since you would have type that full name in your parameter configuration since there is no "FullName" parameter in a build configuration.

@eyalbe4 what do you think?

eyalbe4 commented 6 years ago

That makes sense @casz, as long as getBuildTypeExternalId() is unique per project and has an easy to read structure.

jetersen commented 6 years ago

getBuildTypeExternalId() is the external identifier which is the safer one since it is used in teamcity URL's as well.

eyalbe4 commented 6 years ago

Sounds good @casz.

DimaNevelev commented 6 years ago

@casz @pavel-spacil , On top of what you are doing, I will add two new fields - "Build Name in Artifactory" and "Build Number in Artifactory". These fields will override the provided by TeamCity defaults and will effect only Artifactory. This way users will be able to preserve their old build names and will have no need to rename them in Artifactory.

jetersen commented 6 years ago

@DimaNevelev unsure what you mean or how you would do that.

pavel-spacil commented 6 years ago

What @DimaNevelev is talking about was mentioned in some other issue here I think - two custom fields which can be defined for each build configuration where you can define other build name or build number. Could be useful but in our case where we have hundreds of build names reported in Art the automated way is better. @casz to your question, yes, my changes will not cause loosing the link for older builds.

The ultimate solution which I can think of is to have "build ID" and "build display name" as two fields on Artifactory. Then "build type external id" from TeamCity could be the "build ID" on Artifactory and "build display name" should choose the user by his preferences - some global plugin settings on TeamCity. What I see also as a needed change is to always store full link to build on TeamCity including build number. When you rename the build configuration now you will loose the connection between TC and ART, same applies also to "build type external ID" which is not constant for the build configuration lifetime. Also please note that build can change its own build number during the run so what plugin stores to runner parameters at the start could not be the same value what is used to construct the link on result page.

TCAgentUser commented 6 years ago

@pavel-spacil Actually I think we (someone) should extend the plugin to listen for changes to the externalid and then call the artifactory rest api rename function if any of the build steps have upload spec defined.

Externalid, is for the most part static after build configuration creation, but can over a long period, especially if restructuring your build configs, change.

E.g. we even still have configs in our installation that follow the oldest teamcity naming scheme "bt###"

I am uncertain though how to hook up on change events, but I assume it is possible.

BR Lars J

pavel-spacil commented 6 years ago

Sorry for the offtopic comment. @TCAgentUser yes, it's possible, check interface jetbrains.buildServer.serverSide.ProjectsModelListener where method buildTypeExternalIdChanged is.

DimaNevelev commented 6 years ago

Hi all, FYI, the pull request was released in version 2.7.1. @casz, Thank you for the contribution.