You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/07/05 20:52:15 UTC
[GitHub] [pulsar-client-go] pgier opened a new pull request, #802: [oauth2] Remove oauth2 go.mod and go.sum
pgier opened a new pull request, #802:
URL: https://github.com/apache/pulsar-client-go/pull/802
### Motivation
When the oauth2 sub-directory was added in PR #313 there wasn't any justification given for creating a separate sub-module instead of just treating the oauth2 sources as a normal directory. Since the oauth2 module does not have a separate release cycle, treating it as a sub-module just adds unnecessary complexity.
### Modifications
Removed `go.mod` and `go.sum` from the oauth2 sub-directory. Updated the main `go.mod` and `go.sum` to included the necessary dependencies.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] tisonkun commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1214405143
And we may add some doc (README.md under oauth2?) to say this situation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] nodece commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1214885039
> @nodece thanks for your validation!
>
> Interesting. I ever thought of you can only add a dep in the top level of `go.mod`.
When using the `go get github.com/apache/pulsar-client-go/oauth2@master`, the `golang` will add top level mod to our project, but it seems to have little effect on us:
```
❯ go get github.com/apache/pulsar-client-go/oauth2@master
go: added github.com/apache/pulsar-client-go v0.9.0-candidate-1.0.20220812163439-934c867b3727
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] pgier commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
pgier commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1215131847
@tisonkun I tested the pulsar-ctl build, and this change has no immediate effect on it. Go modules are tied to specific checksums, so this change would only be noticeable during the next upgrade. The pulsar-ctl go.mod currently also has a `replace` directive which prevents upgrade.
Here are the steps to upgrade, if you want to test:
1. remove these two lines from `go.mod`
```
github.com/apache/pulsar-client-go/oauth2 v0.0.0-20211108044248-fe3b7c4e445b
replace github.com/apache/pulsar-client-go/oauth2 => github.com/apache/pulsar-client-go/oauth2 v0.0.0-20211006154457-742f1b107403
````
2. Get the latest commit of pulsar-client-go
```
go get github.com/apache/pulsar-client-go@934c867b3727fe231100715d04e363992ca5ea38
```
3. Run `go mod tidy`
4. Build as normal `make pulsar-ctl`
> Although, such usage may indicate that oauth2 deserves an isolated release lifecycle. You can trade off the choices. I tend to revert this patch and keep oauth2 a separate go module. Because we accept this patch due to "there wasn't any justification given (AFAICT) for creating a separate sub-module", now we have one.
I don't think this justifies the need for a sub-module. Go is pretty efficient in terms of using only the code that it needs, so depending on the root module instead of the oauth2 module should not add any additional code/size to your binary. The oauth2 module has never actually had an isolated release lifecycle (separate release process and version), it's always just been released along with the main project.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] michaeljmarshall merged pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
michaeljmarshall merged PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] pgier commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
pgier commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1179432763
I did a bit of testing just to make sure that this change doesn't break existing apps. AFAICT, a normal `go get github.com/apache/pulsar-client-go@latest` possibly followed by `go mod tidy` should work for most users without any additional changes needed.
In some cases, users may see an `ambiguous import` error after upgrading.
```
github.com/apache/pulsar-client-go/oauth2: ambiguous import: found package github.com/apache/pulsar-client-go/oauth2 in multiple modules
```
The fix for this is to remove the indirect dependency line from go.mod, and the run `go mod tidy`.
```
github.com/apache/pulsar-client-go/oauth2 v0.0.0-20220630195735-e95cf0633348 // indirect
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] tisonkun commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1215146003
@pgier thanks for your explanation! I'll try to learn how go.mod works nowadays in details :)
Actually, such an assumption prevents me reuse a package inner inside vitess and I'm happy to see there's no restriction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] pgier commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
pgier commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1177760671
The main module contained a newer version of `golang.org/x/oauth2` dependency than the oauth2 sub-module, so I had to update some of the tests to match the output of the newer dependency. This type of dependency mismatch will be avoided in the future using a single module defined in the root directory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] tisonkun commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1214893682
@nodece as long as the whole codebase is small and it isn't polluted in the linking stage, it's fine.
Although, such usage may indicate that oauth2 deserves an isolated release lifecycle. You can trade off the choices. I tend to revert this patch and keep oauth2 a separate go module. Because we accept this patch due to "there wasn't any justification given (AFAICT) for creating a separate sub-module", now we have one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] tisonkun commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1217262908
@pgier I read your comment now. And it seems my statement above holds:
> As long as the whole codebase is small and it isn't polluted in the linking stage, it's fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] nodece commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1214535935
> IIRC [streamnative/pulsar-ctl](https://github.com/streamnative/pulsarctl/blob/4e4557d3e67a5e678e80863fed85af7ff04e0314/go.mod) uses oauth2 only and provides an admin API. It will be affected due to this patch and it proves that there're existing packages depends on the isolated releases.
>
> cc @nodece
Thanks for telling me, that changes should break the pulsarctl.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] pgier commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
pgier commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1183406943
@liangyuanpeng Thanks for the feedback, I have added some info to the README about the possible error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] tisonkun commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1214404692
IIRC [streamnative/pulsar-ctl](https://github.com/streamnative/pulsarctl/blob/4e4557d3e67a5e678e80863fed85af7ff04e0314/go.mod) uses oauth2 only and provides an admin API. It will be affected due to this patch and it proves that there're existing packages depends on the isolated releases.
cc @nodece
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] tisonkun commented on pull request #802: [oauth2] Remove oauth2 go.mod and go.sum
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #802:
URL: https://github.com/apache/pulsar-client-go/pull/802#issuecomment-1214575511
@nodece thanks for your validation!
Interesting. I ever thought of you can only add a dep in the top level of `go.mod`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org