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

FISH-788: DirConfigSource should not log severe messages if unused. #5104

Merged
merged 2 commits into from
Jan 29, 2021

Conversation

poikilotherm
Copy link
Contributor

Description

During QA after merging #5007, @lprimak stumbled over a SEVERE
log message during deploys, triggered by the non-existing directory
of DirConfigSource. The directory config value has had a default
before, also the docs say otherwise.

The level issue has been fixed by checking only default config value
being used, resulting in an INFO level message. The logic has been
refactored not to use an exception, but Optional API.

Important Info

Blockers

None.

Testing

New tests

Batteries included.

Testing Performed

Usual unit tests.

Testing Environment

  • AdoptOpenJDK (build 11.0.9+11)
  • Linux, Fedora 33
  • Maven 3.6.3

Documentation

During QA after merging payara#5007, @lprimak stumbled over a SEVERE
log message during deploys, triggered by the non-existing directory
of DirConfigSource. The directory config value has had a default
before, also the docs say otherwise.

The level issue has been fixed by checking only default config value
being used, resulting in an INFO level message. The logic has been
refactored not to use an exception, but Optional API.

- Docs will be fixed to include the formerly hidden default value
- Part of payara#5006
@pdudits
Copy link
Contributor

pdudits commented Jan 29, 2021

Jenkins test please

Copy link
Contributor

@pdudits pdudits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with apps that use and don't use MP Config.

  • INFO error message when default directory doesn't exist -- I'd go for FINE though
  • no message when directory exists
  • error when non-default directory doesn't exist

As requested by @pdudits, reducing the log level to FINE
if the directory targeted by default value (secrets) is not present.
@pdudits
Copy link
Contributor

pdudits commented Jan 29, 2021

Jenkins test please

@lprimak
Copy link
Contributor

lprimak commented Jan 29, 2021

jenkins test

@lprimak
Copy link
Contributor

lprimak commented Jan 29, 2021

the tests failed due to a known bug (which I am fixing)

@lprimak lprimak merged commit dbf30ab into payara:master Jan 29, 2021
@poikilotherm poikilotherm deleted the 5006-enhance-secretdir branch January 30, 2021 00:32
lprimak pushed a commit that referenced this pull request Feb 5, 2021
…5104)

* fix(mpconfig): make DirConfigSource less disrupting if not configured

During QA after merging #5007, @lprimak stumbled over a SEVERE
log message during deploys, triggered by the non-existing directory
of DirConfigSource. The directory config value has had a default
before, also the docs say otherwise.

The level issue has been fixed by checking only default config value
being used, resulting in an INFO level message. The logic has been
refactored not to use an exception, but Optional API.

- Docs will be fixed to include the formerly hidden default value
- Part of #5006

* style(mpconfig): reducing log level in DirConfigSource.findDir()

As requested by @pdudits, reducing the log level to FINE
if the directory targeted by default value (secrets) is not present.
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.

3 participants