You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "abollini (via GitHub)" <gi...@apache.org> on 2024/05/31 21:48:58 UTC
[PR] CONFIGURATION-847 property with an empty string value was not processed [commons-configuration]
abollini opened a new pull request, #431:
URL: https://github.com/apache/commons-configuration/pull/431
This is an attempt to fix https://issues.apache.org/jira/browse/CONFIGURATION-847
The bug was visible in the CI of our open source project, see
https://github.com/DSpace/DSpace/actions/runs/9252818238/job/25451214046#step:4:4916
`Could not resolve placeholder 'epo.consumerKey' in value "${epo.consumerKey}"`
where `epo.consumerKey` was indeed defined in our property configuration as `epo.consumerKey = `
Using a copy of the modified `ConfigurationPropertySource` class in our project solve the issue, see https://github.com/DSpace/DSpace/actions/runs/9322228360/job/25662845056?pr=9605
but we still have an error on some Integration Test that seems to be still related to all these changes, so I'm investigating further see https://github.com/DSpace/DSpace/actions/runs/9322228360/job/25662845391?pr=9605
This failure seems to be related to an health check that is conditioned by an undefined property in our configuration
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CONFIGURATION-847 property with an empty string value was not processed [commons-configuration]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #431:
URL: https://github.com/apache/commons-configuration/pull/431#issuecomment-2143190656
## [Codecov](https://app.codecov.io/gh/apache/commons-configuration/pull/431?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `85.71429%` with `1 lines` in your changes are missing coverage. Please review.
> Project coverage is 89.05%. Comparing base [(`ad9eaa0`)](https://app.codecov.io/gh/apache/commons-configuration/commit/ad9eaa0199524c603bee36483f93b2b2431295e6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`f31382a`)](https://app.codecov.io/gh/apache/commons-configuration/commit/f31382a5a6abea92fa5e491e8f48eae55e9a044a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 206 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/commons-configuration/pull/431?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...iguration2/spring/ConfigurationPropertySource.java](https://app.codecov.io/gh/apache/commons-configuration/pull/431?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fcommons%2Fconfiguration2%2Fspring%2FConfigurationPropertySource.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY29uZmlndXJhdGlvbjIvc3ByaW5nL0NvbmZpZ3VyYXRpb25Qcm9wZXJ0eVNvdXJjZS5qYXZh) | 85.71% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/commons-configuration/pull/431?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #431 +/- ##
============================================
- Coverage 89.09% 89.05% -0.04%
- Complexity 3536 3554 +18
============================================
Files 183 183
Lines 9635 9661 +26
Branches 1192 1202 +10
============================================
+ Hits 8584 8604 +20
- Misses 772 774 +2
- Partials 279 283 +4
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/commons-configuration/pull/431?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CONFIGURATION-847 property with an empty string value was not processed [commons-configuration]
Posted by "abollini (via GitHub)" <gi...@apache.org>.
abollini commented on PR #431:
URL: https://github.com/apache/commons-configuration/pull/431#issuecomment-2143172959
after further investigation, I'm convinced that this PR should solve the issue. The residual problem on our open source project are related to the impossibility to use our "patched" version of the ConfigurationPropertySource in the spring boot context, so the application.properties is still affected by the bug in CONFIGURATION-846 leading to the strange IT failure. More details on our project issue https://github.com/DSpace/DSpace/pull/9605#issuecomment-2143171280
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CONFIGURATION-847] Property with an empty string value was not processed [commons-configuration]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #431:
URL: https://github.com/apache/commons-configuration/pull/431#issuecomment-2145901666
PR merged to git master. Build in https://repository.apache.org/content/repositories/snapshots/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CONFIGURATION-847 property with an empty string value was not processed [commons-configuration]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #431:
URL: https://github.com/apache/commons-configuration/pull/431#issuecomment-2143218987
CC: @kbarlowgw, @tdonohue
Please review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CONFIGURATION-847 property with an empty string value was not processed [commons-configuration]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #431:
URL: https://github.com/apache/commons-configuration/pull/431
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CONFIGURATION-847 property with an empty string value was not processed [commons-configuration]
Posted by "kbarlowgw (via GitHub)" <gi...@apache.org>.
kbarlowgw commented on PR #431:
URL: https://github.com/apache/commons-configuration/pull/431#issuecomment-2145804710
Sounds good to me. I've run it through our code and see no issues.
On Mon, Jun 3, 2024 at 1:30 PM Tim Donohue ***@***.***> wrote:
> ***@***.**** commented on this pull request.
>
> 👍 For my purposes, this looks correct. There's only a single behavior
> change in this code which ensure commons-configuration v2.10.x is again
> "backwards compatible" with v2.9.0.
>
> Essentially, in 2.9.0, if you had an empty string property (e.g. my.property
> = ) this was seen as an empty string ("") in Spring Boot. However, if the
> property was unspecified, this was seen as null in Spring Boot.
>
> The code in this PR looks correct to me as it now correctly differentiates
> an unspecified property from an empty string property.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-configuration/pull/431#pullrequestreview-2094484285>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BBG2EVFOXBQGQOTVZOBFUN3ZFSR2RAVCNFSM6AAAAABITQCBIOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOJUGQ4DIMRYGU>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org