Skip to content

Add java-cfenv reconfiguration jar #76

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Add java-cfenv reconfiguration jar #76

wants to merge 1 commit into from

Conversation

dyroberts
Copy link

@dyroberts dyroberts commented Feb 13, 2020 •
edited
Loading

  • Classic reconfiguration jar remains in its own module
  • New reconfiguration jar...
    • uses java-cfenv to set cloud profile and property source
    • this initial implementation only adds vcap_application properties in 'cloud' property source, not vcap_services, and does not maintain connectors' property naming convention
    • drops bean replacement functionality
    • instead leverages java-cfenv-boot for assorted auto-configuration of bound services
    • had to add the java-cfenv-boot's spring.factories entries to the spring.factories file in this new jar

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 13, 2020 •
edited
Loading

CLA Check
The committers are authorized under a signed CLA.

@dyroberts dyroberts force-pushed the dyroberts/recon-cfenv branch from 72f6e13 to 3fd2bb8 Compare February 13, 2020 15:05
@dyroberts
Add java-cfenv reconfiguration jar
0963a23 
 - Classic reconfiguration jar remains in its own module
 - New reconfiguration jar...
   - uses java-cfenv to set cloud profile and property source
   - this initial implementation only adds vcap_application properties in 'cloud' property source, not vcap_services, and does not maintain connectors' property naming convention
   - drops bean replacement functionality
   - instead leverages java-cfenv-boot for assorted auto-configuration of bound services
   - had to add the java-cfenv-boot's spring.factories entries to the spring.factories file in this new jar
@dyroberts dyroberts force-pushed the dyroberts/recon-cfenv branch from 3fd2bb8 to 0963a23 Compare February 13, 2020 21:24
@dyroberts dyroberts requested a review from nebhale February 13, 2020 21:25
@dyroberts
Copy link
Author

Regarding the lack of a fully populated 'cloud' property source, java-cfenv basic usage has this case covered using SpEL. https://github.com/pivotal-cf/java-cfenv#use-with-spring

@dyroberts
Copy link
Author

It would be nice to have a cleaner solution to the spring.factories file; but I'm not sure what that is. With this implementation, we're stuck updating the file here anytime java-cfenv-boot's version changes.

@scottfrederick
Copy link
Contributor

The cloud properties provided by Connectors were also confusing w.r.t. their similarity to Spring Boot's vcap properties (which came along after Connectors was introduced). I'm in favor of eliminating the cloud properties to reduce this confusion but it could cause pain for users who rely on them currently.

@scottfrederick
Copy link
Contributor

It would be nice to have a cleaner solution to the spring.factories file; but I'm not sure what that is. With this implementation, we're stuck updating the file here anytime java-cfenv-boot's version changes.

Why would this file need to change with java-cfenv-boot versions? I don't see anything version-specific in there.

JBP auto-reconfiguration has always supported an intentionally limited set of connection types, so it wouldn't need to change when new connection types are added to java-cfenv-boot.

@dyroberts
Copy link
Author

Thanks @scottfrederick! I was thinking the same about the cloud property source. Regarding spring.factories file, I just meant that there's the possibility that some java-cfenv version in the future might require an update here. Glad to hear you're ok with this approach.

This was referenced Feb 18, 2020
Closed
Closed
@dyroberts
Copy link
Author

@nebhale Any idea when you might have a chance to look at this? :)

@nebhale nebhale changed the base branch from master to main June 17, 2020 17:49
@scottfrederick scottfrederick mentioned this pull request Jul 16, 2020
Closed
@dmikusa dmikusa self-requested a review June 24, 2021 14:21
@dmikusa
Copy link
Contributor

dmikusa commented Mar 31, 2022

Cross-posting here for awareness: cloudfoundry/java-buildpack#779 (comment)

@dmikusa
Copy link
Contributor

dmikusa commented May 27, 2022

Closing as we have decided to deprecate & remove auto-configuration versus trying to move forward with it.

@dmikusa dmikusa closed this May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nebhale nebhale Awaiting requested review from nebhale

@dmikusa dmikusa Awaiting requested review from dmikusa

Assignees
No one assigned
Labels
scheduled
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dyroberts @scottfrederick @dmikusa