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

Incorrect description in the README #457

Closed
kenfinnigan opened this issue Oct 16, 2019 · 7 comments
Closed

Incorrect description in the README #457

kenfinnigan opened this issue Oct 16, 2019 · 7 comments
Assignees
Labels
clarification 🏆 A clarification (PR) or request for clarification (issue) in the spec or API

Comments

@kenfinnigan
Copy link
Contributor

The project README has the following statement in the third paragraph:

Under some circumstances, some data sources may change dynamically. The changed values should be fed into the client without the need for restarting the application. This requirement is particularly important for microservices running in a cloud environment. The MicroProfile Config approach allows to pick up configured values immediately after they got changed.

As the current specification doesn't describe the ability to handle dynamically changing values, it is probably a good idea to remove this statement.

@Emily-Jiang
Copy link
Member

Both spec and Javadoc on ConfigProperty annotation mentions the retrieval of the dynamic config property.

//Injects a Provider for the value of myprj.some.dynamic.timeout property to
//resolve the property dynamically. Each invocation to Provider#get() will
//resolve the latest value from underlying Config.
//The existence of configured values will get checked during startup.
//Instances of Provider are guaranteed to be Serializable.
@Inject
@ConfigProperty(name="myprj.some.dynamic.timeout", defaultValue="100")
private javax.inject.Provider timeout;

We left how the runtime handles the dynamism unspeced on purpose.

@kenfinnigan
Copy link
Contributor Author

I'd missed that reference in searching yesterday.

Not sure if there are other issues related to this, but I think the spec needs to move away from leaving it undefined otherwise it becomes a huge gray area where implementations can do completely different things and users get unexpected behavior if they migrate between them

@Emily-Jiang
Copy link
Member

Thanks for your feedback, Ken! Points taken. Let's add a couple of sentences in the spec to clearly spell it out.

@ljnelson
Copy link
Contributor

Related: #369, #370, #371, #395, #431

@Emily-Jiang Emily-Jiang self-assigned this Oct 25, 2019
Emily-Jiang added a commit to Emily-Jiang/microprofile-config that referenced this issue Nov 22, 2019
Signed-off-by: Emily Jiang <emijiang6@googlemail.com>
Emily-Jiang added a commit to Emily-Jiang/microprofile-config that referenced this issue Nov 28, 2019
Signed-off-by: Emily Jiang <emijiang6@googlemail.com>
@Emily-Jiang
Copy link
Member

Since we have not reached a conclusion, I'll move this issue out of the MP Config 1.4 milestone.

@Emily-Jiang Emily-Jiang removed this from the MP Config 1.4 milestone Jan 21, 2020
@dmlloyd
Copy link
Contributor

dmlloyd commented Mar 4, 2020

Also related: #432 #304

@dmlloyd dmlloyd added the clarification 🏆 A clarification (PR) or request for clarification (issue) in the spec or API label Mar 4, 2020
Emily-Jiang added a commit that referenced this issue Jul 21, 2020
* #457 add a section to make dynamic config source more visible

Signed-off-by: Emily Jiang <emijiang6@googlemail.com>

* #457 address review comments

Signed-off-by: Emily Jiang <emijiang6@googlemail.com>

* update format and fix typo

Signed-off-by: Emily Jiang <emijiang6@googlemail.com>

* tidy up

Signed-off-by: Emily Jiang <emijiang6@googlemail.com>

* address comments

Signed-off-by: Emily Jiang <emijiang6@googlemail.com>
@Emily-Jiang
Copy link
Member

fixed by #478

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification 🏆 A clarification (PR) or request for clarification (issue) in the spec or API
Projects
None yet
Development

No branches or pull requests

5 participants