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

feature(config-api): Make MPCONFIG in TranslatedConfigView debugable / FISH-1079 #5129

Merged
merged 4 commits into from
Feb 24, 2021

Conversation

poikilotherm
Copy link
Contributor

@poikilotherm poikilotherm commented Feb 16, 2021

Description

Closes #5126

Microprofile Config based variable substitution in TranslatedConfigView:

  1. When no value can be found and no default has been given, a warning
    should be printed, so we have a useful error message. Important e. g.
    to track causes when DB connections fail, etc.
  2. When a value has been found, it might be the wrong one due to wrong
    ordinal config, human mistake, etc etc. Enabling debugging
    with the TranslatedConfigView will reveal the origin of a value.
    (Thx, MicroProfile Config API 2.0!!!)

Important Info

This is a demo to outline how this could be done. Might need polishing.

Testing

New tests

None yet.

Testing Performed

None yet, except for the test cases already present.

Testing Environment

AdoptJDK 11.0.9, Maven 3.6, Linux (Fedora 33)

Documentation

None yet.

Notes for Reviewers

None so far.

1. When no value can be found and no default has been given, a warning
   should be printed, so we have a useful error message. Important e. g.
   to track causes when DB connections fail, etc.
2. When a value has been found, it might be the wrong one due to wrong
   ordinal config, human mistake, etc etc. Enabling debugging
   with the TranslatedConfigView will reveal the origin of a value.

Closes payara#5126
@AlanRoth AlanRoth added PR: CLA CLA submitted on PR by the contributor Type: Community Contribution labels Feb 17, 2021
@AlanRoth AlanRoth self-assigned this Feb 17, 2021
@AlanRoth AlanRoth changed the title feature(config-api): Make MPCONFIG in TranslatedConfigView debugable feature(config-api): Make MPCONFIG in TranslatedConfigView debugable / FISH-1079 Feb 17, 2021
@AlanRoth
Copy link

jenkins test please

Copy link

@AlanRoth AlanRoth left a comment

Choose a reason for hiding this comment

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

Just a quick change, to better align with our code conventions

Rename variable

Co-authored-by: Alan <[email protected]>
Copy link
Member

@Pandrex247 Pandrex247 left a comment

Choose a reason for hiding this comment

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

Spacing

m3.reset(stringValue);
Logger.getAnonymousLogger().fine("Found property '"+matchValue+"' in source '"+cValue.getSourceName()+"' with ordinal '"+cValue.getSourceOrdinal()+"'");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Logger.getAnonymousLogger().fine("Found property '"+matchValue+"' in source '"+cValue.getSourceName()+"' with ordinal '"+cValue.getSourceOrdinal()+"'");
Logger.getAnonymousLogger().fine("Found property '" + matchValue + "' in source '" + cValue.getSourceName() + "' with ordinal '" + cValue.getSourceOrdinal() + "'");

@AlanRoth
Copy link

jenkins test please

@AlanRoth AlanRoth merged commit 0e01683 into payara:master Feb 24, 2021
@poikilotherm
Copy link
Contributor Author

Oh! That went smooth... So I guess the idea was a good fit. Thanks for fixing the style issues.

@AlanRoth I'll add some docs for this in the docs repo.

JamesHillyard pushed a commit to JamesHillyard/Payara that referenced this pull request Sep 17, 2021
feature(config-api): Make MPCONFIG in TranslatedConfigView debugable / FISH-1079
JamesHillyard pushed a commit to JamesHillyard/Payara that referenced this pull request Sep 17, 2021
…5126-mpconfig-msg

feature(config-api): Make MPCONFIG in TranslatedConfigView debugable / FISH-1079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: CLA CLA submitted on PR by the contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MPCONFIG variable substitution does not enforce useful error messages / FISH-1079
3 participants