Clarify how source ordinal applies with an active profile #782
Conversation
|
Can one of the admins verify this patch? |
|
@poikilotherm you need to sign ECA form first. Please click on the Details above and follow instructions there. |
Was already on it 😉 Successfully signed and re-validated just now. Please let me know what you think about the extended spec. Thank you very much for looking into this @Emily-Jiang ! |
|
Thank you @poikilotherm for the PR! Can you also add a couple of sentence on the spec to state the behavior? |
Hi @Emily-Jiang thanks for the feedback. Unfortunately, I'm not sure what you are referring to? Is there some other place in the spec that I should edit besides the profile details that needed clarification? Any hints are much appreciated! |
Sorry for not being clear. I have added more detailed thoughts inline. Will you be able to take at this PR? |
|
@Emily-Jiang thank you for your additional feedback! I rephrased a little as you suggested. Please let me know if this is better now. I already worked on adding a new TCK test locally, but struggeling to find a property not altered by other tests, so it can be used here. Will continue coding, maybe push something preliminary, too, so y'all could have a look. |
A config source with higher ordinal should still override a profiled value from a lower ordinal source with a non-profiled property name.
|
@Emily-Jiang @radcortez @smillidge I just added the new TCK test to this PR. If you want to see it in action, I took notes how to execute the test in addition to the others: https://notes.desy.de/iDjhL8UESlO-_RGNKLDRZQ As expected, Payara fails to be spec compliant, while SmallRye is compliant. |
tck/src/main/java/org/eclipse/microprofile/config/tck/profile/OverrideConfigProfileTest.java
Outdated
Show resolved
Hide resolved
tck/src/main/java/org/eclipse/microprofile/config/tck/profile/OverrideConfigProfileTest.java
Outdated
Show resolved
Hide resolved
|
@poikilotherm can you address the above comments so that I can merge your PR? |
Please excuse my slow response time @Emily-Jiang - I'm on vacation. Applied changes as requested. Thank you again for looking into this! |
|
@poikilotherm Can you fix the following format issue?
|
…n with codebase standard
|
@Emily-Jiang done! 😄 |
|
I will squash and merge your PR @poikilotherm and deal with the format with a separate PR if the build still fails. |
|
Thank you very much @Emily-Jiang ! Much appreciated! Glad this is fixed now. |
Closes #781
This pull request tries to rephrase parts of the profile on property level spec to clarify how ordinal of sources and the profiled vs. unprofiled values work together.