Adding environment variable support to build request #68
Conversation
|
Just a note that failure is expected until restclient is updated. |
| public static class DescriptorImpl extends Descriptor<NameValuePair> { | ||
| public String getDisplayName() { | ||
| return "Name/Value Pairs"; | ||
| } |
There was a problem hiding this comment.
Can the user add a name value pair with empty string ... if so, for consistency sake, we could add checks either here or in the DescriptorImpl for OpenShiftBuilder that ensures something is at least set and we don't go down the env var path when we don't need to.
There was a problem hiding this comment.
Hey - I see where applyEnvVar trims and checks if the name is not empty before storing (don't remember if that was there before or not). Was that your attempt to address this comment?
I was suggestion that you add validation methods to the DescriptorImpl for the name/value pair field (there are example in OpenShiftBuilder) and if you find issues, you can notify the user while he is constructing the build step.
| step.getEnvVars().add( new NameValuePair( entry.getKey().toString(), entry.getValue().toString() ) ); | ||
| } | ||
| } else { | ||
| LOGGER.log(Level.WARNING, "Environment variables must be specified as a map. Received: " + envObject.getClass().getName() ); |
There was a problem hiding this comment.
throwing an exception of some sort (i don't have strong preference between a runtime exception and a custom built exception) seems prudent here so that the error shows up in the job log; otherwise, the job may succeed without the env vars he was expecting and he may not think to double check the jenkins log
There was a problem hiding this comment.
Will change to UnsupportedOperationException.
| @@ -0,0 +1,3 @@ | |||
| <div> | |||
There was a problem hiding this comment.
can't tell from the git review tool ... did you link all the help files to the analogous locations for the dsl resources ... they are consumed by the pipeline grooy snippet generator thing
There was a problem hiding this comment.
They are symlinked now.
|
@gabemontero Should be ready for another once over. |
|
Oh, and I see I forgot to say "thanks" for fixing misspellings and formatting in the readme :-) |
jupierce
force-pushed
the
addenvs
branch
2 times, most recently
from
c25be34 to
6f8440c
Compare
In order to support environment variable feature request: https://trello.com/c/Yl4qomsw/878-5-additional-jenkins-plugin-enhancement-requests-from-github .
Depends on openshift/openshift-restclient-java#217
ptal @gabemontero @bparees
Random changes:
DSL Syntax Example:
openshiftBuild( env: [ test:'value', 'longenv1': 'env2'], apiURL: 'https://10.13.137.85:8443' ....
Build Step Visualization:
