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

Add JobQueue.scheduler_configuration and add corresponding warnings #3913

Merged
merged 7 commits into from Oct 16, 2023

Conversation

Bibo-Joshi
Copy link
Member

@Bibo-Joshi Bibo-Joshi commented Oct 1, 2023

Proposal for closing #3837

Makes the config values used by JobQueue publically accessible so that users can use them in addition to their custom values. Also adds warnigs that explain what this is about.

@rejedai, would you mind also having a look at this, given that you reported the issue?

  • Added .. versionadded:: NEXT.VERSION, .. versionchanged:: NEXT.VERSION or .. deprecated:: NEXT.VERSION to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
  • Created new or adapted existing unit tests
  • Documented code changes according to the CSI standard <https://standards.mousepawmedia.com/en/stable/csi.html>__
  • Added myself alphabetically to AUTHORS.rst (optional)
  • Added new classes & modules to the docs and all suitable __all__ s
  • Checked the Stability Policy <https://docs.python-telegram-bot.org/stability_policy.html>_ in case of deprecations or changes to documented behavior

closes #3837

Copy link
Member

@lemontree210 lemontree210 left a comment

Choose a reason for hiding this comment

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

just a micro-typo

telegram/ext/_jobqueue.py Outdated Show resolved Hide resolved
telegram/ext/_jobqueue.py Outdated Show resolved Hide resolved
Bibo-Joshi and others added 3 commits October 4, 2023 10:04
Co-authored-by: Dmitry K <58207913+lemontree210@users.noreply.github.com>
Co-authored-by: Dmitry K <58207913+lemontree210@users.noreply.github.com>
@lemontree210
Copy link
Member

Please make sure you delete me from authors before merging, otherwise I'll be known for co-authoring a commit to master just because I changed one letter :)

Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

Changes look good to me. However codecov is complaining about untested lines. Is it really not possible to test them?

@harshil21
Copy link
Member

macos py3.12 test consistently failing, but the errors aren't exactly related to this PR?

@Bibo-Joshi
Copy link
Member Author

Mh, codecov doesn't complain in their UI and I'm also pretty certain that the tests cover everything. Maybe the annotations just stuck from the initial runs …

TBH I don't recall why the macos tests failed. Might have been due to codecov upload failure. I'm rerunning.

Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

alright, test pass and code looks covered by the tests 👍🏽

@Bibo-Joshi Bibo-Joshi merged commit f67e8c0 into master Oct 16, 2023
23 checks passed
@Bibo-Joshi Bibo-Joshi deleted the jobstore-configure-warning branch October 16, 2023 18:25
@github-actions github-actions bot locked and limited conversation to collaborators Oct 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] JobQueue.set_application drop jobstores field
3 participants