Thanks for all your comments. I address each one in line. > Regarding the `Close` method, I think it's exceptional. It's designed to return no error so that users can call `defer client.Close()` [1]. To process errors in `Close()`, we can panic in the implementation and let users use `recover` to catch the panic.
My initial idea was to implement the `io.Closer` interface to allow users to free up resources in a general way. After investigating this discussion: https://github.com/golang/go/issues/40483, I think that the close method for Pulsar resources should not return an error. The client should ensure all resources are released when closed, regardless of whether any error has occurred. So this makes sense to me. > I’d add one more item for consideration: using WithX() methods instead of a strict for configurations: https://github.com/apache/pulsar-client-go/issues/68 Thanks for sharing this issue. It makes sense to me. > We can add a method with context, so like: CloseWithContext. > We can still follow the above way, so like: CloseWithError. This would lead to inconsistency in API definitions, making them appear disorganized. For instance, some methods accept a context but don't end with "WithContext.". Same for the `WithError`. This also adds complexity to the API, resulting in a poor user experience. Moreover, we should actually deprecate the old APIs, which would make names like CloseWithCtx and FlushWithCtx seem odd. > In the ConsumerOptions, we use the `internal` package, this is incorrect, when you set the value for `BackoffPolicy`, you will see this error `Use of the internal package is not allowed`. For internal package, please see https://go.dev/doc/go1.4#internalpackages Yes. This issue https://github.com/apache/pulsar-client-go/issues/1187 is actually tracking on it. > This will increase our maintenance times. We maintain two versions of the API, but they could share the same internal code. I don't think this will add much complexity. On the contrary, it would make our API maintenance clearer and easier. The approach mentioned above, using WithContext and WithError, is actually also increasing the maintenance workload. I believe the complexity and workload it brings is not less than maintaining two versions, v1 and v0, of our API. BR, Zike Yang On Sat, Mar 30, 2024 at 7:34 PM Zixuan Liu <node...@gmail.com> wrote: > > I don't recommend breaking user APIs. There are many ways we can avoid it. > > > 1. We should support passing the context to some IO-related methods > like `Ack`, `Close`, `Flush`, etc, or any other methods. There is an > issue related to this discussion. [2] > > We can add a method with context, so like: CloseWithContext. > > > 2. Some methods need to return the error. Like `Reader.HasNext`, > `Close`. This is to adhere to Golang's error-handling standards. > > We can still follow the above way, so like: CloseWithError. > > > 4. Some APIs need to be refined and require introducing breaking > changes as they could affect user experience otherwise. For example, > this [4] is an issue discussing the redesign of the Backoff Policy > API. > > In the ConsumerOptions, we use the `internal` package, this is incorrect, > when you set the value for `BackoffPolicy`, you will see this error `Use of > the internal package is not allowed`. > For internal package, please see https://go.dev/doc/go1.4#internalpackages > > For incorrect implementation, we can fix this. > > > We can provide a separate package path for v1.x API versions, > maintaining v0.x and v1.x APIs separately. > > This will increase our maintenance times. > > > Matteo Merli <matteo.me...@gmail.com> 于2024年3月30日周六 13:08写道： > > > The plan looks great. > > > > I’d add one more item for consideration: using WithX() methods instead of > > a strict for configurations: > > https://github.com/apache/pulsar-client-go/issues/68 > > > > > > -- > > Matteo Merli > > <matteo.me...@gmail.com> > > > > > > On Fri, Mar 29, 2024 at 5:38 AM Zike Yang <z...@apache.org> wrote: > > > > > Hi, everyone > > > > > > The Pulsar Go Client[0] is now relatively mature. I've also noticed > > > that many people in the community have widely used it in their > > > production environments. However, there's still no official 1.0 > > > version for the Go client. I would like to start a thread to discuss > > > the plan for releasing Go client 1.0.0. > > > > > > According to "Go Module version numbering" [1], there are strict > > > requirements for version management in Golang projects, which means we > > > can't introduce any breaking changes within a major version. Before > > > releasing version 1.0.0, we need to review our API and decide on the > > > finalized API for Go client 1.0.0. > > > > > > I've observed that the current design of the Go client's API still has > > > the following issues: > > > > > > 1. We should support passing the context to some IO-related methods > > > like `Ack`, `Close`, `Flush`, etc, or any other methods. There is an > > > issue related to this discussion. [2] > > > 2. Some methods need to return the error. Like `Reader.HasNext`, > > > `Close`. This is to adhere to Golang's error-handling standards. > > > 3. We should expose errors to users so that users can inspect the > > > types of errors returned. [3] is an issue to discuss about this. > > > 4. Some APIs need to be refined and require introducing breaking > > > changes as they could affect user experience otherwise. For example, > > > this [4] is an issue discussing the redesign of the Backoff Policy > > > API. > > > > > > Additionally, we need to continue standardizing the release process > > > and fixing known issues: > > > 1. Refine the changelog formt [5]. We could try to utilize the tool > > > "go-changelog" [6] to generate the changelog automatically. > > > 2. Refine the release process [7] to adhere the Golang Moduel version > > > standard. We need to clearly define the compatibility relationships > > > between different types of versions. Some processes may need to be > > > adjusted to comply with these version standards. > > > > > > These API changes will inevitably introduce breaking changes. However, > > > we do not want the release of Go client 1.0.0 to cause troublesome > > > impacts on the upgrade process for all our existing users. > > > Inspired by the blog "The Principles of Versioning in Go" [8], I > > > believe we need to follow this principle in the process of releasing > > > 1.0.0 and also for maintaining subsequent versions: We should strive > > > to avoid introducing breaking changes to the existing APIs and > > > behaviors. We aim to reduce the steps needed for users to upgrade to > > > the major version. > > > > > > To achieve this, I would like to suggest the following basic solution: > > > > > > We can provide a separate package path for v1.x API versions, > > > maintaining v0.x and v1.x APIs separately. At the same time, we will > > > deprecate all v0.x APIs. For future major versions like 2.x, 3.x, we > > > will follow this same approach according to Golang's standards. In > > > this way, when users upgrade to 1.0.0, they can gradually modify their > > > code to utilize the new version of API, while still being able to use > > > the features of the old API. We will remove the v0.x API in a later > > > version, perhaps in version 2.0.0. > > > > > > The structure for the v1 package would look like this: > > > ├── go.mod # Note: The v0 APIs and v1 APIs will shared the same go.mod > > > ├── v1 > > > │ └── pulsar > > > │ └── client.go > > > └── pulsar > > > └── client.go > > > > > > I did a small demo. You can check it here: > > > https://github.com/RobertIndie/test-go [9]. > > > > > > In this way, the user could still use `go get > > > github.com/apache/pulsar-client-go@v1.0.0` > > <http://github.com/apache/pulsar-client-go@v1.0.0> > > > <http://github.com/apache/pulsar-client-go@v1.0.0> to upgrade to the > > > v1.0.0 > > > version. And use `import > > > "github.com/apache/pulsar-client-go/v1/pulsar"` to use the new V1 API. > > > And for the future major versions like v2.0.0. The users could use `go > > > get github.com/apache/pulsar-client-go/v2` > > <http://github.com/apache/pulsar-client-go/v2> > > > <http://github.com/apache/pulsar-client-go/v2> to install the client and > > > use `import "github.com/apache/pulsar-client-go/v2/pulsar` > > <http://github.com/apache/pulsar-client-go/v2/pulsar> > > > <http://github.com/apache/pulsar-client-go/v2/pulsar> to use the > > > V2 API. > > > > > > While Golang's versioning standard allows us to introduce breaking > > > changes to the v0.x API, I favor preserving the current API. > > > Considering the resistance our users have towards upgrades, I am more > > > inclined to avoid breaking changes to the existing API. This approach > > > should reduce the impact of upgrades. > > > > > > For our next steps, we could proceed as follows: > > > 1. Discuss and finalize the Go client version strategy to adhere to > > > the "Golang Module version standard"[1] > > > 2. Discuss and finalize the V1 API. We may need a PIP to finalize it. > > > 3. Add the V1 API, develop the necessary features, and address issues > > > based on this new API. > > > 4. Release the Go Client 1.0.0 based on the refined release process. > > > > > > This is currently just a preliminary idea and plan for the release of > > > Go client 1.0. I would like to hear your thoughts and ideas. > > > What are your thoughts on this proposal? What else do you believe we > > > need to do before releasing Go client v1.0.0? Please feel free to add > > > any points I may have missed, and feel free to leave your comments > > > here. > > > > > > Thanks, > > > Zike Yang > > > > > > [0] https://github.com/apache/pulsar-client-go > > > [1] https://go.dev/doc/modules/version-numbers > > > [2] https://github.com/apache/pulsar-client-go/issues/1170 > > > [3] https://github.com/apache/pulsar-client-go/issues/1142 > > > [4] https://github.com/apache/pulsar-client-go/issues/1187 > > > [5] https://github.com/apache/pulsar-client-go/blob/master/CHANGELOG.md > > > [6] https://github.com/hashicorp/go-changelog > > > [7] > > > > > https://github.com/apache/pulsar-client-go/blob/master/docs/release-process.md > > > [8] https://research.swtch.com/vgo-principles > > > [9] https://github.com/RobertIndie/test-go > > > > >
