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

Pass initial configuration to additional configuration override. #2531

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kflorence
Copy link
Contributor

Pull Request Checklist

  • Have you read through the contributor guidelines?
  • Have you signed the Lightbend CLA?
  • Have you added copyright headers to new files?
  • Have you updated the documentation?
  • Have you added tests for any changed functionality?

Fixes

Fixes #2448

Purpose

Allow additionalConfiguration access to the initial application configuration which it may override.

Background Context

I had a use-case where I needed to reference values in the initial application configuration in order to implement my override logic. Currently this would require explicitly compiling the initial application configuration in the same way it is already compiled in LagomApplication inside of my additional configuration implementation. See #2448 for more details.

This PR still requires documentation updates and test coverage. I would also like guidance on deprecation: should we allow both methods to exist? If not, what should be provided in since? I have stubbed it out with 1.6.0 for now.

Copy link
Contributor

@ignasi35 ignasi35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kflorence! Left a few comments.

@kflorence
Copy link
Contributor Author

@ignasi35 I will look into test coverage and documentation soon -- feel free to look over the method documentation to see if it is somewhat aligned with how this should be documented in the Lagom documentation. I am also not intimately familiar with Lagom tests -- so any pointers on where additional tests should go would be helpful (otherwise I will try to figure it out when I have some spare time).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Expose environment configuration in LagomApplication
2 participants