Skip to content
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

remove golang.org/x/net dependency #295

Merged
merged 2 commits into from Mar 1, 2023

Conversation

thaJeztah
Copy link
Contributor

The golang.org/x/net/context package is an alias for stdlib context
since go1.7 (see https://go-review.googlesource.com/72570)

This patch replaces all uses of it for stdlib context.

The second commit renames the internal App Engine "context" type, so that
the import aliases can be removed; happy to either drop that commit, or
to squash it with the first one to reduce code-churn.

The golang.org/x/net/context package is an alias for stdlib context
since go1.7 (see https://go-review.googlesource.com/72570)

This patch replaces all uses of it for stdlib context.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Contributor Author

@jinglundong @jihuin ptal 🤗

@thaJeztah
Copy link
Contributor Author

Hmm.. I'm not familiar with CI on this repo; are these failures related to my changes, or a general issue?

ok  	google.golang.org/appengine/v2	1.013s	coverage: 7.6% of statements
=== RUN   TestBasicAPICalls
ERROR: (dev_appserver) python2: command not found
--- FAIL: TestBasicAPICalls (1.51s)
    instance_test.go:24: NewInstance: unable to find admin server URL
=== RUN   TestContext
ERROR: (dev_appserver) python2: command not found
--- FAIL: TestContext (0.50s)
    instance_test.go:75: NewContext: unable to find admin server URL
=== RUN   TestUsers
ERROR: (dev_appserver) python2: command not found
--- FAIL: TestUsers (0.47s)
    instance_test.go:88: NewInstance: unable to find admin server URL
FAIL

@lufik
Copy link

lufik commented Feb 24, 2023

@jihuin do you have some plans to merge this and release new version?

@thaJeztah
Copy link
Contributor Author

FWIW, looks like the failures are due to python2 missing in CI; probably due to GitHub runners having been updated 🤔

coverage: 7.6% of statements
ok  	google.golang.org/appengine/v2	1.017s	coverage: 7.6% of statements
=== RUN   TestBasicAPICalls
ERROR: (dev_appserver) python2: command not found
--- FAIL: TestBasicAPICalls (2.18s)
    instance_test.go:24: NewInstance: unable to find admin server URL
=== RUN   TestContext
ERROR: (dev_appserver) python2: command not found
--- FAIL: TestContext (0.70s)
    instance_test.go:75: NewContext: unable to find admin server URL
=== RUN   TestUsers
ERROR: (dev_appserver) python2: command not found
--- FAIL: TestUsers (0.64s)
    instance_test.go:88: NewInstance: unable to find admin server URL
FAIL

@jinglundong
Copy link
Collaborator

I don't think the test failures are caused by this PR, but not too sure. We could confirm that by running the same test against mainline/master branch (or a dummy change of the mainline/master branch).

@jihuin or I might be able to take a closer look next week. Meanwhile, if this is urgent, please feel free to verify it as well.

@jinglundong
Copy link
Collaborator

I ran the same test in our corp code base. All three failed tests were skipped when running in corp.

...APPENGINE_DEV_APPSERVER not set
--- SKIP: TestBasicAPICalls (0.00s)

@jinglundong
Copy link
Collaborator

We can deal with the test failures separately. #302

@jinglundong jinglundong merged commit a1ae214 into golang:master Mar 1, 2023
@thaJeztah thaJeztah deleted the stdlib_context branch March 1, 2023 19:51
@thaJeztah
Copy link
Contributor Author

Thanks!

sg0hsmt added a commit to sg0hsmt/appengine that referenced this pull request Mar 9, 2024
Use standard context instead of golang.org/x/net/context, and remove dependency.

refs: golang#295
sg0hsmt added a commit to sg0hsmt/appengine that referenced this pull request Apr 10, 2024
Use standard context instead of golang.org/x/net/context, and remove dependency.

refs: golang#295
kennethye1 pushed a commit that referenced this pull request Apr 10, 2024
Use standard context instead of golang.org/x/net/context, and remove dependency.

refs: #295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants