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

API visibility - all non-public APIs should be in “implementation” package #27113

Closed
Tracked by #27112
saragluna opened this issue Feb 15, 2022 · 8 comments
Closed
Tracked by #27112
Assignees
Labels
azure-spring All azure-spring related issues Client This issue points to a problem in the data-plane of the library.

Comments

@saragluna
Copy link
Member

saragluna commented Feb 15, 2022

In the review process of Sprig Cloud Azure 4.0, several classes are considered non-public APIs or chosen to expose later. We agree to move them to the "implementation" package.

  • The *ClientBuilderFactory classes in the spring-cloud-azure-service package.
  • The *ConfigurationProperties classes in the spring-cloud-azure-autoconfigure package (we will change the visibility of these classes).
@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 15, 2022
@saragluna saragluna self-assigned this Feb 15, 2022
@saragluna saragluna added azure-spring All azure-spring related issues Client This issue points to a problem in the data-plane of the library. labels Feb 15, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 15, 2022
@saragluna saragluna moved this to Todo in Spring Cloud Azure Feb 15, 2022
@saragluna
Copy link
Member Author

saragluna commented Feb 15, 2022

The *ClientBuilderFactory classes have been refactored in #26409.

@saragluna
Copy link
Member Author

For the *ConfigurationProperties classes, I've tried to make their visibility to package level, but it won't fit for most cases:

  • We use the AzureComosProperties in spring-cloud-azure-actuator for the Cosmos health indicator.
  • We use StorageBlobProperties in Event Hubs autoconfiguration for checkpoint store.
  • We use the EventHubsProperties in the resource manager configuration.

So this solution won't work.

@lmolkova
Copy link
Member

@saragluna thanks for the info! would it be possible to explore an implementation package option that allows keeping shared classes accessible for other packages?

@saragluna
Copy link
Member Author

Hi @lmolkova, moving the *ConfigurationProperties classes to the implementation package will introduce another problem, for our *Autoconfiguration classes will reference these classes in their constructors.

@lmolkova
Copy link
Member

Hi @lmolkova, moving the *ConfigurationProperties classes to the implementation package will introduce another problem, for our *Autoconfiguration classes will reference these classes in their constructors.

It can still be done with constructor injection (constructor would stay package-private) or am I missing something?

@saragluna saragluna moved this from Todo to In Progress in Spring Cloud Azure Feb 18, 2022
@saragluna saragluna moved this from In Progress to Pending Review in Spring Cloud Azure Feb 18, 2022
@saragluna
Copy link
Member Author

I will try to move them.

@saragluna
Copy link
Member Author

@lmolkova the pr has been merged.

@saragluna
Copy link
Member Author

Closing this issue now for all related classes have been moved to impl packages.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
azure-spring All azure-spring related issues Client This issue points to a problem in the data-plane of the library.
Projects
Archived in project
Development

No branches or pull requests

2 participants