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

SPEC-282: Introduce JVM option to turn off variable support outside config #4132

Merged
merged 1 commit into from
Aug 8, 2019

Conversation

pdudits
Copy link
Contributor

@pdudits pdudits commented Aug 8, 2019

Performing variable expansion in descriptors is not spec compliant, therefore it needs to be switched off for TCK tests. The system property is called fish.payara.substitution.disable and needs to be set to true in order to disable the feature.

Some of the expansions need to be done regardless of the setting, therefore the original method was split into two.

Performing variable expansion in descriptors is not spec compliant,
therefore it needs to be switched off for TCK tests. The system property
is called fish.payara.substitution.disable and needs to be set to true in
order to disable the feature.
Some of the expansions need to be done regardless of the setting,
therefore the original method was split into two.
@pdudits pdudits requested review from arjantijms and jbee August 8, 2019 07:36
Copy link
Contributor

@jbee jbee left a comment

Choose a reason for hiding this comment

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

Nothing to remark on code level. Couldn't follow related discussion during standup on how to do this correctly domain-wise.

result = (T) TranslatedConfigView.getTranslatedValue(result);
if (type == String.class) {
result = (T) TranslatedConfigView.expandValue((String) result);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the else case handled elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getTranslatedValue does transformation only on Strings. I think the story is, that it inherited too general signature, when it was extracted from the interceptor-style method invoke before 2011.

@jGauravGupta
Copy link
Contributor

jenkins test please

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