Skip to content

Add type annotation on concurrency/threads.py#7808

Merged
auvipy merged 2 commits intocelery:masterfrom
Kludex:typing/concurrency/threads
Oct 17, 2022
Merged

Add type annotation on concurrency/threads.py#7808
auvipy merged 2 commits intocelery:masterfrom
Kludex:typing/concurrency/threads

Conversation

@Kludex
Copy link
Copy Markdown
Contributor

@Kludex Kludex commented Oct 15, 2022

Comment on lines -43 to -44
# TODO use a public api to retrieve the current number of threads
# in the executor when available. (Currently not available).
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This will never happen.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 15, 2022

Codecov Report

Base: 89.59% // Head: 89.55% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (4f57abc) compared to base (64f7e89).
Patch coverage: 63.15% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7808      +/-   ##
==========================================
- Coverage   89.59%   89.55%   -0.04%     
==========================================
  Files         128      128              
  Lines       15877    15888      +11     
  Branches     2117     2119       +2     
==========================================
+ Hits        14225    14229       +4     
- Misses       1423     1429       +6     
- Partials      229      230       +1     
Flag Coverage Δ
unittests 89.53% <63.15%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
celery/concurrency/thread.py 75.75% <58.82%> (-19.90%) ⬇️
celery/worker/request.py 95.70% <100.00%> (+<0.01%) ⬆️
celery/app/backends.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.


__all__ = ('TaskPool',)

if TYPE_CHECKING:
Copy link
Copy Markdown
Contributor Author

@Kludex Kludex Oct 16, 2022

Choose a reason for hiding this comment

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

Everything inside this block should have been ignored by codecov, according to

"if TYPE_CHECKING:"

Why isn't?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the config is of pre mypy era of celery, so we can adjust that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't know what needs to be adjusted on codecov. 👀

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

btw codecov is buggy

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to open a PR removing it, and calculate the coverage exclusively with coverage?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I saw some projects moved to that, you can come with that

@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Oct 17, 2022

@auvipy anything else that I can do for this PR? I can clarify anything needed here.

@auvipy auvipy merged commit 6723791 into celery:master Oct 17, 2022
@auvipy auvipy added this to the 5.3 milestone Oct 17, 2022
@Kludex Kludex deleted the typing/concurrency/threads branch October 17, 2022 08:58
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.

2 participants