-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
spanner: remove appengine-specific numChannels. #2513
spanner: remove appengine-specific numChannels. #2513
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change itself looks good to me. What is the exact reason that we can drop this now?
From the issue description, it seems that we somehow must do this for Go 1.9 because App Engine used to support that. Now, the standard env only supports 1.12+ and 1.11 so that we don't have to do this. But from what I can see, the flexible env still supports 1.9 (https://cloud.google.com/appengine/docs/go). |
In https://github.com/googleapis/google-cloud-go#go-versions-supported, we say:
Now, I am a bit confused. In the flexible env of Google App Engine, it still supports 1.9 as its oldest version. @skuruppu Do you think we should merge this? This issue has been in the queue for almost 1 year. |
@broady Do you have any thought about this? Are we safe to remove them? |
@broady Just a follow-up with this PR. Do you have any thoughts? |
Sorry, didn't see this. LGTM. |
In terms of safety, https://cloud.google.com/appengine/docs/deprecations/ states Go 1.9 was shut down June 30, 2020, so there's not a concern there. Also 🎉 ! |
Fixes: #1553