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
Unify jvm memory settings validation & improve upload error message #2111
Unify jvm memory settings validation & improve upload error message #2111
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2111 +/- ##
============================================
+ Coverage 33.58% 33.69% +0.11%
- Complexity 2774 2782 +8
============================================
Files 418 419 +1
Lines 29782 29789 +7
Branches 3779 3780 +1
============================================
+ Hits 10001 10037 +36
+ Misses 18887 18855 -32
- Partials 894 897 +3
Continue to review full report at Codecov.
|
Pull Request Test Coverage Report for Build 4331
💛 - Coveralls |
@burgerkingeater are you able to review this, too? |
@juhoautio can you help me better understand this PR? Why would it help the described issue? |
If there's an empty value like
So it doesn't help much because it doesn't even tell the name of the problematic property. This PR improves it by returning a better error message ie. one that includes the property name. Currently user would need to grab the stack trace, get azkaban source code, and find the line, to know which property caused the upload failure. |
import com.google.common.base.Preconditions; | ||
import org.apache.commons.lang.StringUtils; | ||
|
||
public class MemConfValue { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be applied to other properties? If so, can we make this universal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How? I guess these are the only properties that azkaban parses expecting this pattern.
@burgerkingeater ..good to merge? |
Users shouldn't need to go view Azkaban source to find out why project upload failed in this kind of a case (from azkaban mailing list):