High-level thoughts.

[flowchartsman]

This repo could use some reorganization that would make it more ergonomic to use:

• pulsar-admin-go is an unimportable identifier, whereas admin is non-specific.
  • consider renaming the root module to pulsaradmin, and move the contents of pkg/admin into the repository root
• in general, this package doesn't really need any subpackages at all, with the exception of pkg/rest (and maybe pkg/auth) which should probably be in /internal unless there's a compelling reason to expose them beyond auth configuration, which can live in the project root.. Changes like Error improvements #39 would be the way to expose these. The reorganizations in Optimize project layout #3 Decouple config and auth package #4 feel a bit premature, especially since most users will require at least some kind of auth, so why bother making them import another package?
  • move everything from /pkg into project root or /internal
  • expose necessary types for errors/auth/etc in the root module
• You could even eliminate some of the more confusing overlapping configuration values (len(ClientID)>0 bit me in particular) by simply placing
  • placingAuthProvider directly into the config struct and
  • moving the auth.NewAuth* helpers to the top level and shortening their names

Refs on /internal:

• Simple Go project layout with modules
• Use internal packages to reduce your public API surface

Overall, fewer imports make it easier to be productive with your client without needing to jump around docs and code.

Consider the difference between

import (
	pulsaradmin "github.com/streamnative/pulsar-admin-go"
	pulsarauth "github.com/streamnative/pulsar-admin-go/pkg/admin/auth"
	pulsaradmincfg "github.com/streamnative/pulsar-admin-go/pkg/admin/config"
	pulsarutils "github.com/streamnative/pulsar-admin-go/pkg/utils"
)

func main() {
	paClient, err := pulsaradmin.NewClient(&pulsaradmin.Config{
		PulsarAPIVersion:              pulsaradmincfg.V3,
		WebServiceURL:                 webServiceURL,
		BKWebServiceURL:               custerURL,
		AuthPlugin:                    pulsarauth.TokenPluginName,
		AuthParams:                    tokenData,
		TLSTrustCertsFilePath:         `/etc/ssl/certs`,
		TLSEnableHostnameVerification: true,
	})
	// err handle
	err := paClient.Functions().CreateWithURL(&pulsarutils.FunctionConfig{
		Output:     funcResults,
		LogTopic:   funcLog,
		Tenant:     funcTenant,
		Namespace:  funcNamespace,
		Name:       funcName,
		Inputs:     []string{funcInput},
		Runtime:    "GO",
	}, funcURL)
	// err handle
}

and

import (
	pulsaradmin "github.com/streamnative/pulsar-admin-go"
)

func main() {
	paClient, err := pulsaradmin.NewClient(&pulsaradmin.ClientConfig{
		APIVersion:    pulsaradmin.APIV3,
		WebServiceURL: webServiceURL,
		BKURL:         clusterURL,
		AuthProvider:  pulsarAdmin.AuthToken(tokenData),
		TLSConfig: &pulsarAdmin.TLSConfig{
			TrustCertsPath: `/etc/ssl/certs`,
			HostnameVerify: true,
		},
	})
	// err handle
	err := paClient.Functions().CreateFuncWithURL(funcURL, &pulsaradmin.FunctionConfig{
		Output:     funcResults,
		LogTopic:   funcLog,
		Tenant:     funcTenant,
		Namespace:  funcNamespace,
		Name:       funcName,
		Inputs:     []string{funcInput},
		Runtime:    "GO",
	})
	// err handle
}

Which only requires a single package and is very autocomplete-friendly.

[tisonkun]

Hi @flowchartsman,

Thanks for opening this issue with detailed descriptions and the draft PR!

Generally, I agree that your points about exposing "pulsaradmin" and reorganizing packages with Golang idioms a good idea. You can ping me on the PR once they are ready for review.

Also, maybe you're noticed about the recent discussion and vote for donating this project to the Apache Pulsar project, which can be then a part of pulsar-client-go. I'd suggest you take a look at the discussion thread[1][2] and take this possible future into consideration when making reorganizing decisions.

[1] https://lists.apache.org/thread/txpxdkzf4n5dd6bm0goqxfycr69g9gc4
[2] https://lists.apache.org/thread/tb6c10qlkg9fj56qc0ldkwc79j7qm0vc

/cc @ericsyh @maxsxu please check if the code reorg proposal here affects your usages.

[flowchartsman]

The only reason I've marked it as draft is to keep it from getting merged before comment, but beyond integration testing with testcontainers, I have no other major rewrites I'd like to do. Aside from some outstanding questions on the role of the dedicated namespace types (as opposed to simply strings), I am fine with the design as it is. Since I'm familiar with the internals of pulsar-client-go, pulsar-admin-go, and pulsarctl, I am also happy to audit both APIs and even prep a PR to merge the project intopulsar-client-go. In fact, I am working on a draft PR to bring testcontainers to that package as we speak, so it would dovetail nicely.

[maxsxu]

Close this in favor of tracking in https://github.com/apache/pulsar-client-go