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

Client configuration add namespace #750

Merged

Conversation

zhaohai666
Copy link
Contributor

Which Issue(s) This PR Fixes

Fixes #issue_id

Brief Description

How Did You Test This Change?

@@ -76,6 +77,7 @@ export abstract class BaseClient {
readonly clientType = ClientType.CLIENT_TYPE_UNSPECIFIED;
readonly sslEnabled: boolean;
readonly #sessionCredentials?: SessionCredentials;
readonly namespace?: string = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

readonly namespace?: string; 即可,可选属性不需要设置空值。

@@ -20,15 +20,17 @@ import { Endpoints } from '../route/Endpoints';
import { RetryPolicy } from '../retry';

export abstract class Settings {
protected readonly namespace: string | "";
Copy link
Contributor

Choose a reason for hiding this comment

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

protected readonly namespace: string;

Copy link
Contributor

@fengmk2 fengmk2 left a comment

Choose a reason for hiding this comment

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

+1

@fengmk2
Copy link
Contributor

fengmk2 commented May 14, 2024

@@ -60,8 +60,11 @@ export class SimpleConsumer extends Consumer {
}
}
this.#awaitDuration = options.awaitDuration ?? 30000;
this.#simpleSubscriptionSettings = new SimpleSubscriptionSettings(this.clientId, this.endpoints,
this.consumerGroup, this.requestTimeout, this.#awaitDuration, this.#subscriptionExpressions);
if (options.namespace) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里给namespace初始化一个空字符串,就不需要多次判空了,proxy侧会兼容空字符串的情况将其认为是默认值

Copy link
Contributor

@guyinyou guyinyou left a comment

Choose a reason for hiding this comment

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

lgtm

@RongtongJin RongtongJin merged commit bc2a112 into apache:master May 17, 2024
13 checks passed
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.

4 participants