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

Naming of configuration values in DefaultClientHeadersFactoryImpl #297

Open
pehala opened this issue Jan 17, 2021 · 7 comments
Open

Naming of configuration values in DefaultClientHeadersFactoryImpl #297

pehala opened this issue Jan 17, 2021 · 7 comments
Labels
Milestone

Comments

@pehala
Copy link

pehala commented Jan 17, 2021

The DefaultClientHeadersFactoryImpl is using configuration key org.eclipse.microprofile.rest.client.propagateHeaders configuration key name, while I would expect it to use mp.rest.client.propagateHeaders so I wanted to ask if microprofile has some guidelines for naming their own config keys and which one of these two options is the preferred one.

@andymc12 andymc12 added this to the Future milestone Mar 4, 2021
@andymc12 andymc12 assigned andymc12 and unassigned andymc12 Mar 4, 2021
@andymc12
Copy link
Contributor

andymc12 commented Mar 4, 2021

Hi @pehala - good question. I don't know if the mp.* naming pattern is official, but it was proposed in MP issue 98 which was about six months after we had introduced the org.eclipse.microprofile.rest.client.propagateHeaders config property key.

It does sound like we should consider changing the property, but that should only be done in a major version release (because it would be a breaking change). Perhaps we could make that change when we switch dependencies to Jakarta EE9 (Jakarta REST 3.0 and CDI 3.0).

@Emily-Jiang
Copy link
Member

The naming convention was not enforced and it is a recommendation for the new property names. I would not recommend to update the existing property names just becasue of the naming convention.

@WhiteCat22
Copy link
Contributor

We discussed this issue on the MP Rest Client planning call today.

The plan is to support both properties but deprecate org.eclipse.microprofile.rest.client.propagateHeaders in the next release.

We will also have to specify that mp.rest.client.propagateHeaders takes priority if both are set.

@Emily-Jiang
Copy link
Member

@WhiteCat22 will take a look at this.

@WhiteCat22
Copy link
Contributor

Upon further investigation, we have decided to defer this issue until eclipse/microprofile#98 is resolved.

@jamezp
Copy link
Contributor

jamezp commented May 31, 2024

+1 IMO we should wait for this until there is a decision at the global level.

@jim-krueger
Copy link
Contributor

jim-krueger commented Jun 4, 2024

@Emily-Jiang, Can you please create a Milestone 4.1 and add this there, removing it from 4.0?

@Emily-Jiang Emily-Jiang modified the milestones: 4.0, 4.1 Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants