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

Add singleton principle to RestService #198

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Claimundefine
Copy link
Contributor

RestService would be created multiple times for both the DekRegistry and SchemaRegistry clients. This would specifically be an issue as the OAuth client would also be created twice, with two tokens to manage.

@Claimundefine Claimundefine requested review from a team as code owners December 4, 2024 18:55
@@ -51,14 +51,15 @@ export interface ClientConfig {
const toBase64 = (str: string): string => Buffer.from(str).toString('base64');

export class RestService {
private static instance: RestService;
private client: AxiosInstance;
private baseURLs: string[];
private oauthClient?: OAuthClient;
private oauthBearer: boolean = false;

constructor(baseURLs: string[], isForward?: boolean, axiosDefaults?: CreateAxiosDefaults,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would normally be private, but this would break the jest mocks and RestService is not exposed to the user regardless.

@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@airlock-confluentinc airlock-confluentinc bot force-pushed the DGS-19297-create-singleton-for-rest-client branch from b75429d to 2924fa7 Compare December 4, 2024 19:07
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.

1 participant