Skip to content

Adding new test ServerMyTestApplicationTests to show the behavior of spring.config.import added in application.yml vs set as -Dspring.config.import #1839

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

Closed
wants to merge 2 commits into from

Conversation

nagsuchandra
Copy link

Difference in behavior with spring.config.import added in application.yml vs set as -Dspring.config.import

…spring.config.import added in application.yml va set as -Dspring.config.import
@pivotal-issuemaster
Copy link

@nagsuchandra Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@nagsuchandra Thank you for signing the Contributor License Agreement!

@spencergibb
Copy link
Member

Can you describe why you have added this?

@nagsuchandra
Copy link
Author

@spencergibb I added the test to show the issue . application.yml have spring.config.import set in there changes the behavior than when we set it as -Dspring.config.import .

@codecov
Copy link

codecov bot commented Mar 16, 2021

Codecov Report

Merging #1839 (313871a) into master (35f219c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1839   +/-   ##
=========================================
  Coverage     78.18%   78.18%           
  Complexity     1302     1302           
=========================================
  Files           164      164           
  Lines          4785     4785           
  Branches        649      649           
=========================================
  Hits           3741     3741           
  Misses          781      781           
  Partials        263      263           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35f219c...313871a. Read the comment docs.

@spencergibb
Copy link
Member

Ok. You could've done that in a personal repo. The behavior is out of spring cloud's ability to control since that is decided by spring boot. What are your expectations with this PR?

@nagsuchandra
Copy link
Author

nagsuchandra commented Mar 16, 2021

Ok. You could've done that in a personal repo. The behavior is out of spring cloud's ability to control since that is decided by spring boot. What are your expectations with this PR?

Ok . .Don't merge the PR .. I will leave it in my repo only.
However the issue is visible when spring config server is in the picture, hence I added the test under spring-cloud-config-sample's test cases.

@spencergibb
Copy link
Member

So the point of this was pointing out the difference in behavior? I'm really trying to understand motivation beyond the sample.

@nagsuchandra
Copy link
Author

nagsuchandra commented Mar 16, 2021

So the point of this was pointing out the difference in behavior? I'm really trying to understand motivation beyond the sample.

The point was mainly to show the difference in behavior - yes that is right .
And that difference in behavior is actually breaking some of the applications in our environment and as a workaround, we had to set spring.config.use-legacy-processing to true for the time being.

You mentioned earlier that it is an issue of spring boot , however the issue creates an impact in the context of config server and ordering of properties gets messed up ( please see the details of the issue for further clarity).
If -Dspring.config.import is provided, the properties defined in remote config repo have higher precedence .
However if spring.config.import is added in application.yml of the app, the properties defined in remote config repo no longer maintain the higher precedence.

Are you suggesting that there is no fix that can be added from spring cloud side ? IMO the issue falls in the middle of spring boot and spring cloud config .

@spencergibb
Copy link
Member

Closing and moving conversation to #1838

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

Successfully merging this pull request may close these issues.

4 participants