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

Deprecate WS_1004_NO_STATUS_RCVD and WS_1005_ABNORMAL_CLOSURE #1580

Merged
merged 5 commits into from Apr 22, 2022

Conversation

Kludex
Copy link
Sponsor Member

@Kludex Kludex commented Apr 7, 2022

Closes #1578

This PR has the same idea as django/asgiref#272.

The idea here is to deprecate two constants using the PEP 562. Unfortunately I need to use a backport for Python 3.6 - but I guess we'd want to release this for Python 3.6 before removing the Python 3.6 support.

NOTE: After this PR gets in, a patch release is needed.

@Kludex Kludex requested a review from a team April 8, 2022 08:16
Copy link
Member

@aminalaee aminalaee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kludex Is this worth the effort as Python 3.6 is already EOL and there's plan to remove support in Starlette too?
I mean as you said we can hold it after Python 3.6 is removed.

@Kludex
Copy link
Sponsor Member Author

Kludex commented Apr 10, 2022

I mean as you said we can hold it after Python 3.6 is removed.

I didn't say that, at least I didn't mean it. I mean that we should fix it before stop supporting Python 3.6, so at least the last version has it right.

@Kludex Kludex mentioned this pull request Apr 15, 2022
2 tasks
@Kludex Kludex added this to the Version 0.19.1 milestone Apr 21, 2022
Copy link
Member

@euri10 euri10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, thanks @Kludex

@adriangb
Copy link
Member

So the idea here is to cut 1 release with those code then immediately remove it right?

So:

  1. Merge this PR. Cut a patch release w/ Python 3.6 support.
  2. New PR that deletes the back port code.
  3. Cut a minor release dropping Python 3.6 support.

Am I getting this right?

@Kludex
Copy link
Sponsor Member Author

Kludex commented Apr 21, 2022

Yes.

@Kludex
Copy link
Sponsor Member Author

Kludex commented Apr 22, 2022

The backport removal will be on the PR that drops Python 3.6.

@Kludex Kludex merged commit 9329160 into master Apr 22, 2022
@Kludex Kludex deleted the deprecate-ws-codes branch April 22, 2022 04:55
@Kludex Kludex mentioned this pull request Oct 4, 2022
11 tasks
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.

BUG: Incorrect WebSocket status code 1005 description, missing status code 1006
4 participants