Be able to specify the log Level when system property is not found #2646

Merged
merged 1 commit into from Nov 28, 2016

Conversation

5 participants
@batmat
Member

batmat commented Nov 24, 2016

Some properties are more important than others. One may for example want to log a WARNING instead of the current default CONFIG when some property can not be found.

@reviewbybees

@reviewbybees

This comment has been minimized.

Show comment
Hide comment
@reviewbybees

reviewbybees Nov 24, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

String value = System.getProperty(key); // keep passing on any exceptions
if (value != null) {
- if (LOGGER.isLoggable(Level.CONFIG)) {
- LOGGER.log(Level.CONFIG, "Property (system): {0} => {1}", new Object[] {key, value});
+ if (LOGGER.isLoggable(logLevelIfAbsent)) {

This comment has been minimized.

@alvarolobato

alvarolobato Nov 25, 2016

Member

In this case should be logLevel, you are logging when you find the property not when it is missing.

@alvarolobato

alvarolobato Nov 25, 2016

Member

In this case should be logLevel, you are logging when you find the property not when it is missing.

This comment has been minimized.

@batmat

batmat Nov 25, 2016

Member

Right. Fixed. Switch back to simply logLevel for every places, since anyway ...IfAbsent is not the whole picture: the log will also be used for impossible conversions, not just when it's not specified and so on.

@batmat

batmat Nov 25, 2016

Member

Right. Fixed. Switch back to simply logLevel for every places, since anyway ...IfAbsent is not the whole picture: the log will also be used for impossible conversions, not just when it's not specified and so on.

Be able to specify the log Level when system property is not found or…
… invalid

Some properties are more important than others. One may for example
 want to log a WARNING instead of the current default CONFIG
when some property can not be found, or has an invalid format
(like a string, when asking for an Integer).
@daniel-beck

This comment has been minimized.

Show comment
Hide comment
@daniel-beck

daniel-beck Nov 25, 2016

Member

Will there be a followup PR with actual application? So far I have a difficult time imagining when this would be useful.

Member

daniel-beck commented Nov 25, 2016

Will there be a followup PR with actual application? So far I have a difficult time imagining when this would be useful.

@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Nov 25, 2016

Member

@daniel-beck it's linked above already: #2645

Member

batmat commented Nov 25, 2016

@daniel-beck it's linked above already: #2645

@oleg-nenashev

Looks good to me

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
Member

oleg-nenashev commented Nov 27, 2016

🐝 and @reviewbybees done

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@alvarolobato

This comment has been minimized.

Show comment
Hide comment
Member

alvarolobato commented Nov 28, 2016

🐝

@batmat batmat merged commit a05d942 into jenkinsci:master Nov 28, 2016

2 checks passed

Jenkins This pull request looks good
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@batmat batmat deleted the batmat:add-level-to-SystemProperties branch Nov 28, 2016

daniel-beck added a commit that referenced this pull request Dec 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment