Seed error reporting improvements #87
Conversation
| return item | ||
| } | ||
| else { | ||
| println " WARNING: parent ${parentName} is not a folder but of type '${item?.class}''" |
There was a problem hiding this comment.
item?: the null-safe operator is not required here because you checked item == null before.
| if (item == null) { | ||
| println " WARNING: parent folder ${parentName} not found" | ||
| return null | ||
| } |
There was a problem hiding this comment.
Please move the else if to this line for consistency, same with the else below.
| @@ -43,7 +43,7 @@ def createItem(ItemGroup parent, String name, FilePath file) { | |||
| parent.createProjectFromXML(name, file.read()) | |||
| createdItems++ | |||
| } else { | |||
| println " Could not create project in parent of type ${parent.class}" | |||
| throw new RuntimeException("Parent is not a folder type ModifiableViewGroup but of type '${parent?.class}'") | |||
There was a problem hiding this comment.
Commit message: "...are created..."
These exceptions will be catched in updateJobs so they will not make the job fail.
There was a problem hiding this comment.
I left the typo in the commit message so I don't need to force push the branch. Let me know what you prefer.
Yes the exception will be caught, but then the totalFailures var will be incremented and this raises an exception after all jobs are updated.
There was a problem hiding this comment.
I prefer the Gerrit-like rebase workflow because it creates a nicer history (I'm not a fan of PR fixup commits because they leave a broken state in the history), so force pushing the PR branch is fine.
| @@ -42,6 +42,8 @@ def createItem(ItemGroup parent, String name, FilePath file) { | |||
| if (parent instanceof ModifiableViewGroup) { | |||
| parent.createProjectFromXML(name, file.read()) | |||
| createdItems++ | |||
| } else if (parent == null) { | |||
| throw new RuntimeException("Parent is null. This happens when the parent folder could not get created, which can happen if a job with the name of the parent folder already exists. To solve this rename the job or the parent folder.") | |||
There was a problem hiding this comment.
- Please stick to the 120 character line length limit.
- "if a job with the name of the parent folder already exists": This is only one of the possible reasons, maybe add "which can happen for example..." to avoid confusion in case there was another problem.
- If you add the null check the null-safe operator should be removed from
parent?below.
| @@ -59,6 +61,8 @@ def createView(ItemGroup parent, String name, FilePath file) { | |||
| if (parent instanceof ModifiableViewGroup) { | |||
| ((ModifiableViewGroup) parent).addView(View.createViewFromXML(name, file.read())) | |||
| createdViews++ | |||
| } else if (parent == null) { | |||
| throw new RuntimeException("Parent is null. This happens when the parent folder could not get created, which can happen if a job with the name of the parent folder already exists. To solve this rename the job or the parent folder.") | |||
There was a problem hiding this comment.
- Please stick to the 120 character line length limit.
- "if a job with the name of the parent folder already exists": This is only one of the possible reasons, maybe add "which can happen for example..." to avoid confusion in case there was another problem.
- If you add the null check the null-safe operator should be removed from
parent?below.
- Mark exception output clearly with ERROR - Indent summary so it stands out in console output view
This helps diagnose issues when Seed job fails.
Make sure the Seed job goes red in case not all jobs are created as expected.
Make the error more clear in case a job exists with the same name as a folder, and offer a possible solution.
stefanverhoeff
force-pushed
the
seed-errors-improve
branch
from
236cb67 to
6a8051a
Compare
|
Fixes now rebased into original commits |
|
Thanks @stefanverhoeff ! |
Several improvements to make sure the Seed job fails on errors related to parent folders, and make error reporting more clear.