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 support for subscription expiration time namespace settings #1254

Merged

Conversation

klevy-toast
Copy link
Contributor

@klevy-toast klevy-toast commented Jul 19, 2024

Fixes #1253

Motivation

Adds support for the get / set / delete subscriptionExpirationTime namespace settings.

Modifications

Created new functions in namespace.go that implement support for subscriptionExpirationTime settings

Verifying this change

  • Make sure that the change passes the CI checks.

    • Added integration tests for new client functions

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / GoDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@klevy-toast klevy-toast changed the title Add support for subscription expiration namespace settings Add support for subscription expiration time namespace settings Jul 19, 2024
Copy link
Member

@RobertIndie RobertIndie left a comment

Choose a reason for hiding this comment

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

LGTM. Please fix the conflict

@klevy-toast
Copy link
Contributor Author

Thanks for the review @RobertIndie , I have fixed the merge conflicts

@RobertIndie
Copy link
Member

Please take a look at this compilation error: https://github.com/apache/pulsar-client-go/actions/runs/10045685387/job/27779892242?pr=1254#step:5:30

  Error: pulsaradmin/pkg/admin/namespace_test.go:234:11: undefined: admin

@klevy-toast

@klevy-toast
Copy link
Contributor Author

Thanks, fixed @RobertIndie

@RobertIndie
Copy link
Member

There are still some lint issues. If you are using macos or linux, you can run make lint to check locally.

@klevy-toast
Copy link
Contributor Author

@RobertIndie thanks, fixed.

Is there a way for me to run the build independently? It's a bit annoying that I cannot see what issues are failing the build until a maintainer triggers it.

@RobertIndie
Copy link
Member

Is there a way for me to run the build independently? It's a bit annoying that I cannot see what issues are failing the build until a maintainer triggers it.

You can run make build to build it locally. To check for CI failures, you can open a pull request in your own repository. This allows you to review any CI issues directly.

@RobertIndie RobertIndie merged commit c74460d into apache:master Jul 27, 2024
7 checks passed
@klevy-toast
Copy link
Contributor Author

Is there a way for me to run the build independently? It's a bit annoying that I cannot see what issues are failing the build until a maintainer triggers it.

You can run make build to build it locally. To check for CI failures, you can open a pull request in your own repository. This allows you to review any CI issues directly.

That's great info, can it be added to the contributing guide?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add namespaces subscriptionExpirationTime commands
2 participants