Fix/build user vars plugin #164
Conversation
mvignesh12
commented
Jan 21, 2020
•
mvignesh12
commented
- Fixed Post build query not updating the dashboard with expanded values of variables in additional columns set with build user vars plugin
- Fixed Post-Build Update: WARN: Could not alter table env_dashboard to add column
Syncing latest changes from base repo
…s of variables in additional columns set with build user vars plugin
src/main/java/org/jenkinsci/plugins/environmentdashboard/DashboardBuilder.java
Outdated
Show resolved
Hide resolved
| @@ -172,7 +180,7 @@ public void tearDown(Run<?, ?> build, FilePath workspace, Launcher launcher, Tas | |||
| context.setDisposer(new TearDownImpl()); | |||
| } | |||
|
|
|||
| @SuppressWarnings("rawtypes") | |||
There was a problem hiding this comment.
why did you remove this suppress?
There was a problem hiding this comment.
I've added it back. I removed it during code modifications.
src/main/java/org/jenkinsci/plugins/environmentdashboard/DashboardBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jenkinsci/plugins/environmentdashboard/DashboardBuilder.java
Outdated
Show resolved
Hide resolved
| @@ -260,7 +273,7 @@ private String writeToDB(Run<?, ?> build, TaskListener listener, String envName, | |||
| stat.execute(runQuery); | |||
| } catch (SQLException e) { | |||
| returnComment = "Error running query " + runQuery + "."; | |||
| return returnComment; | |||
| return returnComment + e; | |||
There was a problem hiding this comment.
This will fail, e is not a string?
There was a problem hiding this comment.
Added it to print the exact exception which I was getting during debugging. Removed it now.
|
Thanks very much for having a crack at this @mvignesh12 - just some nitpicks to get tidied up and I think we can merge this! |
|
Thanks Will. I've updated the code based on your reviews. Please help in merging this PR and release the latest version. |
|
@mvignesh12 great, thank you! I'd like to do a little testing this weekend, but I think this is looking good for a merge here. To be able to do another release of the plugin, we'd like to get this migrated over to the |
