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

Revert removing of SyncToAsync.get_current_task #454

Closed
wants to merge 1 commit into from

Conversation

legau
Copy link
Contributor

@legau legau commented Mar 21, 2024

SyncToAsync.get_current_task was removed in #367 where it was not documented and introduces regressions in our code base

@andrewgodwin
Copy link
Member

This was never a publicly-documented API; may I ask what type of code was relying on it? We can add it back but it's also mostly just dispatching to asyncio.

Copy link
Member

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

@legau This is unused, so at the least you'll want to add a test case covering it to prevent a regression in the future.

You may be better off using asyncio.current_task directly. Is there a reason not to do that? (Other than the obvious undocumented breakage...)

@carltongibson
Copy link
Member

carltongibson commented Mar 21, 2024

Cc @jthorniley FYI (And if you have any thought...)

@legau
Copy link
Contributor Author

legau commented Mar 24, 2024

Hi, this is used in an obscure piece of an internal lib of ours. We are going to replace it by asyncio.current_task, could we make this deprecated for now ?

@carltongibson
Copy link
Member

@legau It's probably easiest if you pin to version 3.7.2 whilst you work on the adjustment. That's a one-line requirements change for you, vs quite a lot of disruption to re-add, release, and immediately deprecate the method, which as Andrew notes was undocumented private API anyway.

@legau
Copy link
Contributor Author

legau commented Mar 26, 2024

I guess it's easier I'm closing the PR. However I disagree on it being a private method, it is a publicly named method of a documented publicly exposed class.

@legau legau closed this Mar 26, 2024
@legau legau deleted the revert-synctoasync-current-task branch March 26, 2024 10:34
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

3 participants