Replaced CARTRIDGE_CLONE_URL from String to ExtensibleChoiceParameter… #32
Conversation
projects/jobs/jobs.groovy
Outdated
| def loadCartridgeCollectionJob = workflowJob(cartridgeManagementFolderName + "/Load_Cartridge_Collection") | ||
|
|
||
|
|
||
| def loadCartridgeCollectionJob = workflowJob(cartridgeManagementFolderName + "/Load_Cartridge_Collection")/var/jenkins_home/userContent |
There was a problem hiding this comment.
Should this "/var/jenkins_home/userContent" be here?
There was a problem hiding this comment.
Does this mean Load_Cartridge has to be run on master as it depends on the file system?
projects/jobs/jobs.groovy
Outdated
|
|
||
| return catridges_list; | ||
| } | ||
| catch (Exception e) { |
There was a problem hiding this comment.
Can we be a bit more explicit about the exceptions being caught. This may aid debugging in the future.
|
Implemented @kramos's request for a package manager system for cartridges. This PR now depends on the following PRs: |
|
Testing:
|
There was a problem hiding this comment.
This looks good to me. A few minor quirks:
- May need to be careful of the dependency on the snakeyaml package (in the unlikely event they deprecate it)
- I have noticed a weird error sometimes in choice parameters populated by dynamic scripts, where the first time you run a job via "Build with Parameters", the script does not run and the dropdown appears empty, and then you basically configure the job, but not make any changes and then save, and then the parameters are magically populated. Have we encountered this issue on a fresh ADOP instance with your platform-management repository loaded in?
Other than that, it looks good to me!
I'm assuming this PR has a dependency on Accenture/adop-jenkins#31 and Accenture/adop-docker-compose#188 to be merged in first.
projects/jobs/jobs.groovy
Outdated
|
|
||
| if (URLS == null) { | ||
| return ['[ERROR] CARTRIDGE_SOURCES Jenkins environment variable has not been set']; | ||
| println "[ERROR] CARTRIDGE_SOURCES Jenkins environment variable has not been set"; |
There was a problem hiding this comment.
Println should be before the return as the branch will never execute.
Also, if URLS is equal to "" we should probably return.
There was a problem hiding this comment.
Looks good to me (comments given in person on the error message and yaml format).
@dsingh07 I coulnd't replicate this at all. I opened ~30 browser and every single one behaved. Are you able to replicate?
There was a problem hiding this comment.
@kramos It isn't anything to do with the browser session, it's the first time you try and build with parameters on a fresh ADOP Jenkins, where the population script has never been evaluated. I haven't personally tested it with this plugin, but it maybe an issue specific to the plugin I was using since I've only ever seen it happen with Dynamic Choice Parameter.
|
New YAML format:
And small changes to how the YAML is parsed:
|
|
Tested and looks good to me. |
… so that the cartridge list can be dynamically loaded
|
LGTM! One thing that has now occurred to me is that we aren't taking cartridges from Gerrit any more with this, perhaps we need to remove that functionality? Alternatively, we can still do that but place the equivalent cartridges.yml within userContent for people to switch over too if they don't want to come straight from GitHub. |
… so that the cartridge list can be dynamically loaded.
I've added a Groovy script in the ExtensibleChoiceParameter which loads a list of cartridges from "https://raw.githubusercontent.com/Accenture/adop-platform-management/master/cartridges.txt" and presents them to the user.
In addition, every time that list is loaded, a backup of it is saved in "/var/jenkins_home/userContent/cartridges.txt", just in case the list provider is ever down. (Obviously all these parameters can be changed so that they point to different providers, so the list of cartridges won't necessarily be hosted in the same place as the cartridges, as is the case currently with GitHub).
In addition, because we are using the ExtensibleChoiceParameter plug-in, a user can now specify a custom cartridge URL without having to modify the job.