Skip to content

Add command unsubscribe and clearBacklog and command set and get for dispatch and subscribe and replicator rate #68

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
Merged
zymap merged 7 commits into from
Sep 24, 2019
Merged

Add command unsubscribe and clearBacklog and command set and get for dispatch and subscribe and replicator rate #68

zymap merged 7 commits into from
Sep 24, 2019

Conversation

@zymap
Copy link
Member

@zymap zymap commented Sep 18, 2019

Modifications

  • Add command clear-backlog
  • Add command set-dispatch-rate
  • Add command get-dispatch-rate
  • Add command messages-encryption
  • Add command set-replicator-dispatch-rate
  • Add command get-replicator-dispatch-rate
  • Add command set-subscribe-rate
  • Add command get-subscribe-rate
  • Add command set-subscription-auth-mode
  • Add command set-subscription-dispatch-rate
  • Add command get-subscription-dispatch-rate
  • Add command unsubscribe

@sijie sijie added this to the 0.0.1 milestone Sep 18, 2019
sijie
sijie requested changes Sep 18, 2019
View reviewed changes
Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

Also please use consistent naming pattern for the files.

@zymap zymap force-pushed the namespace-rate-setting branch from 91685c6 to 469c48a Compare September 19, 2019 02:02
@sijie sijie mentioned this pull request Sep 19, 2019
Closed
29 tasks
sijie
sijie requested changes Sep 19, 2019
View reviewed changes
Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

I only left one comment for each different type of comment. Please address the similar places based on my comments.

pkg/ctl/namespace/get_subscription_dispatch_rate.go Outdated
func GetSubscriptionDispatchRateCmd(vc *cmdutils.VerbCmd) {
var desc LongDescription
desc.CommandUsedFor = "This command is used for getting subscription message-dispatch-rate for all subscriptions of a namespace."
desc.CommandPermission = "This command requires super-user permissions."
Copy link
Member

@sijie sijie Sep 19, 2019

Choose a reason for hiding this comment

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

why this command needs super-user permissions?

pkg/ctl/namespace/set_dispatch_rate.go
func SetDispatchRateCmd(vc *cmdutils.VerbCmd) {
var desc LongDescription
desc.CommandUsedFor = "This command is used for setting message-dispatch-rate for all topics of a namespace."
desc.CommandPermission = "This command requires super-user permissions."
Copy link
Member

@sijie sijie Sep 19, 2019

Choose a reason for hiding this comment

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

same comment as above

Copy link
Member Author

@zymap zymap Sep 20, 2019

Choose a reason for hiding this comment

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

 protected void internalSetTopicDispatchRate(DispatchRate dispatchRate) {
        log.info("[{}] Set namespace dispatch-rate {}/{}", clientAppId(), namespaceName, dispatchRate);
        validateSuperUserAccess();

In a broker, it will validate super-user access. Is it need to add the spreadsheet?

Copy link
Member

@sijie sijie Sep 24, 2019

Choose a reason for hiding this comment

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

yes please add it to the spreadsheet.

pkg/ctl/namespace/set_dispatch_rate.go Outdated
}

setByByte := Example{
Desc: "Set the message-dispatch-rate by byte <rate> for all topics of namespace <namespace-name>",
Copy link
Member

@sijie sijie Sep 19, 2019

Choose a reason for hiding this comment

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

so why you are adding <> around rate?

Copy link
Member Author

@zymap zymap Sep 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Updated as Set the default message dispatch rate by bytes of a namespace to <rate>

@zymap zymap force-pushed the namespace-rate-setting branch from 469c48a to a18b9f6 Compare September 20, 2019 06:23
@wolfstudy
Copy link
Contributor

wolfstudy commented Sep 23, 2019

@sijie PTAL again

@zymap If you have already resolved the comments, please inform @sijie review this pull request again, thanks.

sijie
sijie approved these changes Sep 24, 2019
View reviewed changes
Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

after fixing my comment, this change is ready to go.

pkg/ctl/namespace/set_dispatch_rate.go
func SetDispatchRateCmd(vc *cmdutils.VerbCmd) {
var desc LongDescription
desc.CommandUsedFor = "This command is used for setting message-dispatch-rate for all topics of a namespace."
desc.CommandPermission = "This command requires super-user permissions."
Copy link
Member

@sijie sijie Sep 24, 2019

Choose a reason for hiding this comment

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

yes please add it to the spreadsheet.

@zymap
Add command unsubscribe and clearBacklog and command set and get for …
2618e5c 
…dispatch and subscirbe and replicator rate

--

*Modifications*

- Add command clear-backlog
- Add command set-dispatch-rate
- Add command get-dispatch-rate
- Add command messages-encryption
- Add command set-replicator-dispatch-rate
- Add command get-replicator-dispatch-rate
- Add command set-subscribe-rate
- Add command get-subscribe-rate
- Add command set-subscription-auth-mode
- Add command set-subscription-dispatch-rate
- Add command get-subscription-dispatch-rate
- Add command unsubscribe
@zymap
Rename file
08df0cf
@zymap
Use This command for doing to replace This command to do
86de782
@zymap
Address comments
ee67f70
@zymap
Address comments
01531ab
@zymap
Address comments
c608400
@zymap
Add license header
ae4c966
@zymap zymap force-pushed the namespace-rate-setting branch from 160e621 to ae4c966 Compare September 24, 2019 06:54
@zymap zymap merged commit 2f6910c into master Sep 24, 2019
@wolfstudy wolfstudy deleted the namespace-rate-setting branch October 11, 2019 06:25
tisonkun pushed a commit to tisonkun/pulsar-client-go that referenced this pull request Aug 15, 2023
@zymap @tisonkun
Add command unsubscribe and clearBacklog and command set and get for …
62d9224 
…dispatch and subscribe and replicator rate (streamnative/pulsarctl#68)

* Add command unsubscribe and clearBacklog and command set and get for dispatch and subscirbe and replicator rate
--

*Modifications*

- Add command clear-backlog
- Add command set-dispatch-rate
- Add command get-dispatch-rate
- Add command messages-encryption
- Add command set-replicator-dispatch-rate
- Add command get-replicator-dispatch-rate
- Add command set-subscribe-rate
- Add command get-subscribe-rate
- Add command set-subscription-auth-mode
- Add command set-subscription-dispatch-rate
- Add command get-subscription-dispatch-rate
- Add command unsubscribe
tisonkun pushed a commit to apache/pulsar-client-go that referenced this pull request Aug 16, 2023
@zymap @tisonkun
Add command unsubscribe and clearBacklog and command set and get for …
1939c6c 
…dispatch and subscribe and replicator rate (streamnative/pulsarctl#68)

* Add command unsubscribe and clearBacklog and command set and get for dispatch and subscirbe and replicator rate
--

*Modifications*

- Add command clear-backlog
- Add command set-dispatch-rate
- Add command get-dispatch-rate
- Add command messages-encryption
- Add command set-replicator-dispatch-rate
- Add command get-replicator-dispatch-rate
- Add command set-subscribe-rate
- Add command get-subscribe-rate
- Add command set-subscription-auth-mode
- Add command set-subscription-dispatch-rate
- Add command get-subscription-dispatch-rate
- Add command unsubscribe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sijie sijie sijie approved these changes

Assignees

@zymap zymap

Labels

None yet

Projects

None yet

Milestone

0.1.0

Development

Successfully merging this pull request may close these issues.

4 participants

@zymap @wolfstudy @sijie