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

Bump io.smallrye.config:smallrye-config from 3.4.1 to 3.4.2 #2372

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Nov 7, 2023

Bumps io.smallrye.config:smallrye-config from 3.4.1 to 3.4.2.

Release notes

Sourced from io.smallrye.config:smallrye-config's releases.

3.4.2

  • #1046 Ignore .env folder in the .env Config provider
  • #1045 Do not rely on order to assert mapping toString result
  • #1043 Case-sensitive EnvProperty
  • #1042 Fix StringUtil.skewer to properly convert camelCase and upper case names
  • #1041 Match dotted dashed property names in Environment Variables
  • #1029 SmallRyeConfigBuilderCustomizer documentation
  • #1028 @​WithDefaults documentation
Commits
  • b0d7f34 [maven-release-plugin] prepare release 3.4.2
  • 13b2b43 Release 3.4.2 (#1047)
  • 9824707 Ignore .env folder in the .env Config provider
  • cba3d94 Do not rely on order to assert mapping toString result
  • aad0659 Case-sensitive EnvProperty
  • bf9969f Fix StringUtil.skewer to properly convert camelCase and upper case names
  • 4709cce Match dotted dashed property names in Environment Variables
  • 92c4fe6 SmallRyeConfigBuilderCustomizer documentation
  • 6c085f9 @​WithDefaults documentation
  • f1dd9ab [maven-release-plugin] prepare for next development iteration
  • See full diff in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

Bumps [io.smallrye.config:smallrye-config](https://github.com/smallrye/smallrye-config) from 3.4.1 to 3.4.2.
- [Release notes](https://github.com/smallrye/smallrye-config/releases)
- [Commits](smallrye/smallrye-config@3.4.1...3.4.2)

---
updated-dependencies:
- dependency-name: io.smallrye.config:smallrye-config
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot added dependencies Pull requests that update a dependency file java Pull requests that update Java code labels Nov 7, 2023
@dependabot dependabot bot requested review from cescoffier and ozangunalp November 7, 2023 21:59
@cescoffier
Copy link
Contributor

@radcortez it breaks again:

Error:  Failures: 
Error:    ConnectorConfigTest.testGetFromEnv:124 
Expecting Optional to contain:
  "used"
but was empty.
Error:    ConnectorConfigTest.testPropertyNames:101 
Expecting actual:
  ["channel-name",
    "at-tr",
    "connector",
    "some-key",
    "attr.2",
    "attr",
    "attr2",
    "attr1",
    "attr4",
    "key"]
to contain exactly in any order:
  ["connector",
    "ATTR1",
    "attr1",
    "attr2",
    "attr.2",
    "ATTR",
    "attr",
    "AT_TR",
    "at-tr",
    "key",
    "SOME_KEY",
    "some-key",
    "SOME_OTHER_KEY",
    "ATTR3",
    "attr4",
    "channel-name"]
but could not find the following elements:
  ["ATTR1", "ATTR", "AT_TR", "SOME_KEY", "SOME_OTHER_KEY", "ATTR3"]

@cescoffier
Copy link
Contributor

@radcortez please do not bump in Quarkus before investigating what's wrong here.

@radcortez
Copy link
Member

No worries, I've noticed it and fixed it with a new version. There is a 3.4.3 available with another fix.

@radcortez radcortez closed this Nov 8, 2023
Copy link
Contributor Author

dependabot bot commented on behalf of github Nov 8, 2023

OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting @dependabot ignore this major version or @dependabot ignore this minor version. You can also ignore all major, minor, or patch releases for a dependency by adding an ignore condition with the desired update_types to your config file.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

@dependabot dependabot bot deleted the dependabot/maven/main/io.smallrye.config-smallrye-config-3.4.2 branch November 8, 2023 14:42
@radcortez
Copy link
Member

Hum, this may be something different. Let me check.

@radcortez
Copy link
Member

Due to some edge cases, I've restricted some environment names matching that made these tests pass, but they now fail. Consider:

export MP_MESSAGING_INCOMING_FOO_AT_TR=value

Because of the keys split in the internal Config, you get a prefix mp.messaging.incoming. and a name foo. The tests look up the internal Config for AT_TR, which prefixes the final key to lookup in the overall Configasmp.messaging.incoming.foo.AT_TR, mixing the dotted format with the environment variable format. If you use, mp.messaging.incoming.foo.at-trorMP_MESSAGING_INCOMING_FOO_AT_TR`, the lookup succeeds as expected.

If I remember correctly, everything you get after mp.messaging.incoming.foo are connector attributes which we control. So if you have a connector attribute called at-tr, the lookup has to be done with that name and not AT_TR (leave the handling of environment variables to the underlying Config.

I'll probably argue that tests that lookup connector attributes in their environment variable format are not correct, especially because Docker (and I've learn that recently, which caused the edge cases), can set environment variables as:

mp.messaging.incoming.foo.AT_TR=foo
MP_MESSAGING_INCOMING_FOO_AT_TR=bar

Which are two completely separate properties with different values.

But other systems like bash only allow you to set MP_MESSAGING_INCOMING_FOO_AT_TR (and case-sensitive variations).

Overall, I think we should be fine with those changes and just remove the tests that check for the environment variable format.

@radcortez
Copy link
Member

Let me also have a look into other scenarios.

@cescoffier
Copy link
Contributor

cescoffier commented Nov 8, 2023 via email

@radcortez
Copy link
Member

We do not control the set of connector attribute. It’s an open set.

It is ok to be an open set.

My comment was that the test generates inconsistent configuration names (which did work before). Even if it worked and someone is using it, we shouldn't allow it.

For instance, if you have the attribute hostname and the connector mp.messaging.incoming.foo, then acceptable lookip configurations should be:

  • mp.messaging.incoming.foo.hostname
  • MP_MESSAGING_INCOMING_FOO_HOSTNAME (env name conversion)

But this shouldn't be accepted:

  • mp.messaging.incoming.foo.HOSTNAME

If we agree with this, are we ok to remove some of these asserts? If not, I guess I'll have to find another way to readd those variations :)

@cescoffier
Copy link
Contributor

Unfortunately, the POSIZ rules seem to be ignored by many popular systems: https://docs.docker.com/engine/reference/builder/#environment-replacement

(Look at the examples: use lowercase).

You can also find references of mixed cases: https://stackoverflow.com/questions/75110404/how-can-i-set-a-lower-case-environment-variable-in-docker-compose-yml.

I don't mind not supporting these cases anymore, but we need to be ready to get issues opened. This should be considered as a breaking change and be documented as such. We will likely have application breakage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file java Pull requests that update Java code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants