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

Reorg #1

Merged
merged 4 commits into from
Jul 27, 2023
Merged

Reorg #1

merged 4 commits into from
Jul 27, 2023

Conversation

flowchartsman
Copy link
Owner

(Mirrored local PR of #streamnative#41)

Motivation

Pursuant to streamnative#40, this is my idea of reorganizing the repo to make it more ergonomic. Marking this as a draft since it is an unsolicited major, breaking change. Hopefully this can serve as a source of discussion.

Features:

  • All features accessible from the root package and a single configuration struct
  • Names have been made more consistent, to aid in grouping and autocomplete
  • Newer V3 API endpoints (currently just functions) are supported by default. I also laid the foundations for incrementing them individually if necessary.
  • Auth providers are easier to create
    • functional-option style helper functions for the builtin auth types allow the user to easily create them without needing to call into docs or use the plugin names.
    • Errors will occur when creating the client
    • Transports are transparently layered and requests are cloned.
    • custom auth providers can also be created, provided they match the function signature.
  • Custom transport as a base is supported, using *http.Transport for a configurable base and only using http.Roundtripper internally.
  • Auth and rest internals are not exposed.
  • Pursuant to Error improvements streamnative/pulsar-admin-go#39, errors are improved
    • errors should be testable using errors.As using the APIErr and ServerErr interface types.
    • The new IsNotFound(error) function can identify errors that are simply the result of something not existing
  • Things that need to be visible should appear in Godoc more clearly, such as the auth plugin names, definitions for the AuthParams. AuthParams that have a KV fallback are also noted in the Godoc.
  • Though there are more files in the root, the organization using the api_ and client_ prefixes help navigation, and the types can be grouped under endpoint if less files are desired.

What's next?

Since I really needed this for work, I will probably continue to work on it in the coming weeks by adding integration testing using the a current Pulsar Image and testcontainers-go The first priority is making sure all of the auth methods work correctly so a mock server like mock-oauth2-server with testcontainers will probably also play a role. Then I would probably start to exercise the various APIs using various authentication methods for better coverage. At some point, I'd also like to experiment with unifying the various function signatures, such as the way topics/list requires the special NamespaceName type, but many other methods take (tenant, namespace). Would love to discuss those things. Also keen to make a special type for topic lists that will present as a unified list, but allow you to break it into partitioned/non-partitioned, etc...

Modifications

See above

Verification

All current tests have been updated.

@flowchartsman flowchartsman merged commit 0f81a7c into master Jul 27, 2023
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