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

Let other services use config cache #4166

Merged
merged 3 commits into from
Dec 17, 2024

Conversation

Michal-Leszczynski
Copy link
Collaborator

This is the first general step toward fixing #3892.
It allows for accessing config cache service from other services.
Actually using config cache service will be implemented in separate PRs.

Ref #3892

@Michal-Leszczynski Michal-Leszczynski force-pushed the ml/export-config-cache-to-svc branch from 3561c16 to dd51843 Compare December 13, 2024 13:32
This is the first general step toward fixing #3892.
It allows for accessing config cache service from other
services. Actually using config cache service will be
implemented in separate PRs.

Ref #3892
It's going to be useful when fixing #3892.

Ref #3892
@Michal-Leszczynski Michal-Leszczynski force-pushed the ml/export-config-cache-to-svc branch from dd51843 to c6a71a5 Compare December 13, 2024 14:03
@Michal-Leszczynski Michal-Leszczynski marked this pull request as ready for review December 13, 2024 14:31
Copy link
Collaborator

@VAveryanov8 VAveryanov8 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Michal-Leszczynski
Copy link
Collaborator Author

@karol-kokoszka @VAveryanov8 sorry guys, I added one more commit to the PR.
When trying to use the changes on other branch, it turned out that I also need to add a cluster to the cluster svc passed to config cache svc. Otherwise, config cache svc would never query this cluster config.

@karol-kokoszka
Copy link
Collaborator

@Michal-Leszczynski I understand this PR just injects the service to other services that are going to use it, right ?
The approach could be to do it in the PR that actually starts consuming this config-cache in other services.

@karol-kokoszka
Copy link
Collaborator

But feel free to merge.

@Michal-Leszczynski
Copy link
Collaborator Author

The approach could be to do it in the PR that actually starts consuming this config-cache in other services.

Yea, but I already implemented a helper method for creating config cache svc in tests in this PR and it wasn't wasn't really working correctly (because of describes problem), so I wanted to keep it as a part of this PR.

@Michal-Leszczynski Michal-Leszczynski merged commit ad742f6 into master Dec 17, 2024
51 checks passed
@Michal-Leszczynski Michal-Leszczynski deleted the ml/export-config-cache-to-svc branch December 17, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants