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

Remove Persistent Topics v3 API - use custom media type instead #14117

Merged
merged 4 commits into from
Feb 4, 2022

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Feb 4, 2022

Fixes #14104

Motivation

  • PR PIP 110: Support Topic metadata - PART-1 create topic with properties #12818 changes for PIP-110 broke the API. A newer admin client cannot be used with an older broker.
  • It’s possible to have the current v2 API for creating persistent topics to handle multiple different payload types
    The challenge seems to be that in the old method, the number of partitions was passed in the payload as a single integer without having a JSON object at all. This makes it harder to extend the existing API, but doesn’t make it impossible.
  • This solution is based on request’s “Content Type”
    Content-Type: application/vnd.partitioned-topic-metadata+json on request, @Consumes("application/vnd.partitioned-topic-metadata+json") on method that handles the new payload type.

Modifications

  • use custom media type application/vnd.partitioned-topic-metadata+json to distinguish the new content payload type

Copy link
Contributor

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

but we should find a way to clearly document this in the documentation
maybe swagger is already well documenting this new media type?

could you please verify ?

@eolivelli eolivelli merged commit 996b301 into apache:master Feb 4, 2022
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
…he#14117)

* Remove Persistent Topics v3 API - use custom media type instead

* Don't pass properties if there are none

* Cleanup remains of topicV3, use old API method when no properties are set

* Fix tests
@tisonkun tisonkun mentioned this pull request Dec 22, 2022
2 tasks
@tisonkun
Copy link
Member

but we should find a way to clearly document this in the documentation
maybe swagger is already well documenting this new media type?

@eolivelli Swagger distinguishes endpoints by verb + path only. It can be a docs issue, see also #18947.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
6 participants