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

Token expanded tags #3

Closed
wants to merge 3 commits into from
Closed

Conversation

rajish
Copy link

@rajish rajish commented Aug 8, 2014

All tokens, build parameters and environment variables placed into the tags string using the usual form of ${variable} are now expanded before sending a message.

Close #2 upon merging this PR.

@cloudbees-pull-request-builder

plugins » flowdock-plugin #3 SUCCESS
This pull request looks good

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@anttipitkanen
Copy link

Sorry for the delay in getting back to you about this pull request! Can you provide an example of tag configuration with expandable variables? I don't seem to be able to get any variables expanded from the tag string (tried with: ${JOB_NAME}, ${BUILD_NUMBER}, etc). Those tags seem to arrive as they are to the Flowdock API.

@rajish
Copy link
Author

rajish commented Sep 22, 2014

@anttipitkanen that looks like a state from before merging the code in this PR. I've been using this modification for some time now and it expands every macro I put there, which among others includes ${BUILD_NUMBER}.

@anttipitkanen
Copy link

I think I found the issue: the token-macro plugin is marked as optional dependency, but in order to send the notification successfully it needs to be installed. To resolve this, there should be some fallback for the cases where the user has not installed the plugin (documentation: https://wiki.jenkins-ci.org/display/JENKINS/Dependencies+among+plugins).

Another way would be marking the plugin as required dependency, but that's not really desirable solution.

On a side note, I would love to avoid the "catch-all" in the notifyFlowdock method since it loses stack traces and other information about exceptions that are really unexpected and should be rather allowed to bubble up. Catching only MacroEvaluationException would be optimal in my opinion since that's something we expect to see and can handle there.

@rajish
Copy link
Author

rajish commented Sep 25, 2014

@anttipitkanen I have applied your suggestions. Please review.

@anttipitkanen
Copy link

A couple of things I found while testing the latest version:

If the token-macro plugin does not exist, you'll run into java.lang.ClassNotFoundException: org.jenkinsci.plugins.tokenmacro.MacroEvaluationException as the try-catch uses it. I guess there's no pretty way around that except for catching just Exception or maybe loading the class manually inside the plugin conditional. I would probably go for the former as it's easier to implement and the "catch-all" only affects the expandAll method call.

The token-macro plugin is fetched and assigned in the constructor which seems to result in value null for me every time with Jenkins v1.580. However, if I move it to the notifyFlowdock just before the try-catch, it finds the plugin nicely and uses it to expand macros.

@anttipitkanen
Copy link

This was implemented without plugin dependencies in pull request #8

@rajish
Copy link
Author

rajish commented Dec 10, 2014

That's slightly different stuff - it expands only environment variables. Token macros are slightly more useful.

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.

Add support for variable expansion in tags
4 participants