> We maintain two versions of the API, but they could share the same internal code.
Thanks for your explanation! +1 Jie crossover <crossover...@gmail.com> 于2024年4月1日周一 22:20写道: > Greet plan. > > We maintain two versions of the API, but they could share the same > > internal code. I don't think this will add much complexity. > > I agree with this. > BTW, there is another issue that needs to be considered. > > https://github.com/apache/pulsar-client-go/issues/919 > > > -- > Best Regards! > crossoverJie > > > Yunze Xu <x...@apache.org> 于2024年4月1日周一 20:46写道: > > > > This would lead to inconsistency in API definitions, making them > > > appear disorganized. > > > > I agree. Using such suffixes is really ugly and much harder to > > maintain IMHO. It only makes people add APIs more casually. They might > > think, oh, don't worry, if this API (e.g. doSomething()) does not make > > sense in future, you can add another API with a suffix (e.g. > > doSomethingInAnotherWay()). > > > > Thanks, > > Yunze > > > > On Mon, Apr 1, 2024 at 8:05 PM Zike Yang <z...@apache.org> wrote: > > > > > > 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> > > > > > <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> > > > > > <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> > > > > > <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 > > > > > > > > > > > > > >