Google-cloud-go: all: support context.Context instead of golang.org/x/net/context.Context?

[rakyll]

The Context type will be in the standard library once Go 1.7 releases. Switch to the context.Context type before we make any API compatibility promises.

/cc @okdave @jba

[okdave]

This is bigger than just trace: it would be weird for the trace API to use 1.7's native Context, but the other APIs to still take Context as their first arg for all the calls.

This is an excellent reason for proposal golang/go#16339 which would let us solve this more gracefully.

But without that, I don't really know what/how to make the change. The only way to support both is to fork every single file into a 1.6 and 1.7 version. And we're committed supporting 1.6 until at least 1.8 is out, so there's a long time for which we'll need both.

[okdave]

(Thinking a little more – maybe we only care about returned Contexts? That would limit the scope a lot but we'd still have a weird inconsistency.)

[admtnnr]

How does this affect, if at all, the user experience for customers also using the google.golang.org/appengine packages?

I wouldn't want to force users to do weird workarounds to trade x/net/context.Contexts for context.Contexts and moving the appengine packages to use the standard context.Context isn't going to be possible (without gri's aliasing proposal) in the near future.

[rakyll]

How does this affect, if at all, the user experience for customers also using the google.golang.org/appengine packages

The suggestion also applies to the appengine packages. I can't see a point leaving appengine packages depending on x/net/context while migrating the cloud ones.

But apart from that consistency between appengine and cloud packages and the migration plan to "context", Go 1.6 promise seems to be a big blocker.

[admtnnr]

Even assuming we didn't support 1.6 on App Engine anymore now that 1.7 is out, we can't just break user code that potentially explicitly references x/net/context.Context. At least that's my opinion. @okdave?

[rakyll]

appengine package migrated from appengine.Context to x/net/context.Context (see https://github.com/golang/appengine/commit/1c3fdc51e1021e4822cf8475c97d3a14a2a6648e). I don't see why "context" migration isn't possible given that it is more trivial if we don't have different API promises now because it is just an import path change.

[okdave]

That's right: we've made some strong commitments on backwards support for App Engine packages. Most of the time, developers won't notice the change but there are defintiely cases where they will need to update their code (datastore.RunInTransaction jumps out in my mind).

I honestly can't see an smooth way forward without some support from the language itself to let us consider the two Context types as equivalent.

One solution (which is really ugly) is two maintain two copies: one for 1.6 with x/net/context and one for 1.7 with context), differentiated by release tags. Since the context usage is so invasive, we'd end up with almost every file completely duplicated, so we'd want a programmatic way to keep them in sync. This would be jaw-droppingly awful, but at least users could control their migration.

[okdave]

@rakyll that change happened when managed VMs / App Engine flex was still in early development (IIRC, we hadn't even opened it up to trusted tester at that stage) so we didn't break any users with that change.

The old appengine.Context type still exists for users using the legacy packages, and we still support it. In fact, deprecating those packages will be a long process and not something we can do without a lot of notice in advance.

[okdave]

(You can actually see that in @dsymonds comment at the bottom of that commit – we hadn't even anticipated using those packages for standard App Engine when the commit landed)

[dsymonds]

Actually it's a little easier. The change from appengine to google.golang.org/appengine was tricky (and will remain tricky to fully remove the former) because the former package is treated as part of the standard library on App Engine. It is included in the prod toolchain, and the deployed app doesn't include a copy of it. If we want to be able to rebuild already-deployed apps, we need to keep an appengine package around.

However, google.golang.org/appengine is not part of the prod toolchain. It is shipped along with deployed apps that use it. The only impact on changing external-facing code in that package (e.g. by switching away from the x/net/context.Context type) is that new deployments would fail. It wouldn't have any impact on already-deployed apps.

You'd need build tags (since we would need to keep the 1.6 -> 1.7 transition smooth), but it wouldn't be as fraught with peril as the previous transition was.

However, I'm not sure how important it is to transition right away. The two context types interoperate (the interfaces are equivalent). It's only an issue if you've got it in a func/interface signature that you're trying to use as a type, and that's not very common.

[rakyll]

The only way we could break code like that is if we could give users a transition period and reasonable deadline. I can't see a good way to achieve that.

To be clear, I am not in the favor of breaking anyone overnight but would like to act on any plan to contribute the resolution of the fragmentation in context types.

The very compatibility issue @adamtanner mentions between appengine and cloud package users will exist between "context" and cloud users. The main reason why it is not a critical issue today is that we don't have any current users of the new "context" package but it is an unavoidable issue in the future. The long term solution would be being on the same page everywhere.

However, I'm not sure how important it is to transition right away. The two context types interoperate (the interfaces are equivalent). It's only an issue if you've got it in a func/interface signature that you're trying to use as a type, and that's not very common.

The only major point is to be on the same page with the rest of the community for compatibility and the timeframe is mostly dependent on how fast the community will act on the new context.Context type. App Engine packages are more isolated from the community-driven libraries and utilities and the cloud packages will likely to face the incompatibility issue sooner.

[otraore]

Any update on this? There are a few crucial libraries I'm unable to use because of this.

[zombiezen]

@otraore Can you link me to code or describe more of what is blocking you about this?

[otraore]

https://github.com/go-chi/chi/issues/105 @zombiezen , If I recall correctly using libraries that use the built in context package doesn't play nicely with appengine.

[zombiezen]

@otraore I see. What you're describing is a different issue than this one, but very closely related. Your issue is that Go on App Engine Standard is on 1.6 (although we have 1.8 in beta right now!), which causes problems when you bring in a library that uses context from the standard library.

This bug is about trying to switch all of the imports of golang.org/x/net/context in the cloud.google.com/go/... packages to use the standard library context. We haven't done this yet because of exactly the problem you have run into. We want these libraries to work correctly on App Engine Standard, so we're using golang.org/x/net/context to workaround.

With the advent of type aliases coming in Go 1.9, golang.org/x/net/context.Context will become an alias for context.Context, so at least for this bug, the issues of importing golang.org/x/net/context will lessen for non-App-Engine users while still supporting Go < 1.8 on App Engine Standard.

[otraore]

Oh I see, thank you for clearing it up. Do you think it's wise to start a project on go1.8 beta for appengine? We'd very much like to use certain libraries but we also want it to run on appengine because it's an amazing platform. The dilemma is how long the beta will last before becoming stable.

[derekperkins]

Go ahead and start on 1.8 beta. It's very unlikely that you'll run into any
issues, and it shouldn't be too long before it makes it into GA.

On Mon, Jul 17, 2017 at 12:49 PM, Ousmane Traore [e-mail]
wrote:

Oh I see, thank you for clearing it up. Do you think it's wise to start a
project on go1.8 beta for appengine? We'd very much like to use certain
libraries but we also want it to run on appengine because it's an amazing
platform. The dilemma is how long the beta will last before becoming stable.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/GoogleCloudPlatform/google-cloud-go/issues/312#issuecomment-315846414,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADbCqg7li3zTFjnzwzEYDrT9z4e5E1M-ks5sO6ywgaJpZM4Jfhla
.

[otraore]

Thanks @derekperkins, will do. Looking forward to 1.8 being stable in the near future.

[anthmgoogle]

It is not clear if this is a specific request for a change any more or whether it can be closed out. @zombiezen can you clarify?

[zombiezen]

IMO, there's no specific action that needs to be taken and this issue should be closed. In Go 1.9, golang.org/x/net/context.Context aliases to context.Context, so this issue will just go away on its own.

[anthmgoogle]

Thanks. Closing.