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.

Sign up for GitHub

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

Jump to bottom

bot: add deprecation notice to SopelWrapper #2521

Merged
merged 1 commit into from
Oct 14, 2023
Merged

bot: add deprecation notice to SopelWrapper #2521

merged 1 commit into from
Oct 14, 2023

Conversation

SnoopJ
Copy link
Contributor

@SnoopJ SnoopJ commented Oct 13, 2023

Description

This changeset adds a deprecation notice to SopelWrapper as the first part of #2460.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make lint and make test)
  • I have tested the functionality of the things this change touches

@SnoopJ SnoopJ added Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon. Housekeeping Code cleanup, removal of deprecated stuff, etc. labels Oct 13, 2023
@SnoopJ SnoopJ added this to the 8.0.0 milestone Oct 13, 2023
@dgw
Copy link
Member

dgw commented Oct 14, 2023

This is… not the right approach. It's too early to warn on every creation of a SopelWrapper object, and doing it this way doesn't show anything in the docs either. This approach also removes SopelWrapper from the docs entirely.

I wrote up the detailed explanation below first, but then decided to summarize it as above in case you're short on spoons.

The Long Version

You can't see it when testing this patch as written, because master's version is 8.0.0.dev0 and prerelease versions are "less than" the same stable version—but this warning will make logs unusable. A lot of SopelWrappers get created during normal bot operation—at least one every time a rule matches and the bot dispatches a callback.

Try changing version and warning_in to 7.1, then start the bot. A normally peaceful startup sequence (one without DEBUG logs enabled, that is) turns into:

$ sopel
Sopel 8.0.0.dev0 (running on Python 3.10.12)
https://sopel.chat/

Loaded config file: /home/dgw/.sopel/default.cfg
[2023-10-13 18:38:58,661] sopel.bot            INFO     - Loading plugins...
[2023-10-13 18:38:58,674] sopel.bot            INFO     - Plugin loaded: calc
[2023-10-13 18:38:58,734] sopel.bot            INFO     - Plugin loaded: search
[2023-10-13 18:38:58,736] sopel.bot            INFO     - Plugin loaded: currency
[2023-10-13 18:38:58,738] sopel.bot            INFO     - Plugin loaded: reload
[2023-10-13 18:38:58,740] sopel.bot            INFO     - Plugin loaded: flipper
[2023-10-13 18:38:58,907] sopel.bot            INFO     - Plugin loaded: twitter
[2023-10-13 18:38:58,911] sopel.bot            INFO     - Plugin loaded: coretasks
[2023-10-13 18:38:58,911] sopel.bot            INFO     - Registered 6 plugins, 0 failed, 33 disabled
[2023-10-13 18:38:58,912] asyncio              DEBUG    - Using selector: EpollSelector
[2023-10-13 18:38:59,047] sopel.irc            INFO     - Connected, initiating setup sequence
[2023-10-13 18:39:00,983] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:00,983] sopel.bot            INFO     - Client capability negotiation list: away-notify, chghost, invite-notify, multi-prefix, sasl, userhost-in-names
[2023-10-13 18:39:01,015] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,017] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,018] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,019] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,020] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,020] sopel.coretasks      INFO     - Capability negotiation ended successfuly.
[2023-10-13 18:39:01,085] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,086] sopel.coretasks      INFO     - Enabled client capabilities: userhost-in-names, away-notify, sasl, chghost, multi-prefix
[2023-10-13 18:39:01,086] sopel.coretasks      INFO     - No custom command to execute.
[2023-10-13 18:39:01,087] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,088] sopel.coretasks      INFO     - Received RPL_MYINFO from server: SopelTest, irc.rizon.io, plexus-4(hybrid-8.1.20)
[2023-10-13 18:39:01,088] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,090] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,091] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,118] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,119] sopel.coretasks      INFO     - Joining 1 channels (with JOIN throttle OFF); this may take a moment.
[2023-10-13 18:39:01,120] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,153] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,154] sopel.coretasks      INFO     - Channel joined: #dgw
[2023-10-13 18:39:01,158] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,158] sopel.coretasks      INFO     - Channel's topic updated: #dgw
[2023-10-13 18:39:01,159] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,192] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,192] sopel.coretasks      INFO     - Updated mode for channel: #dgw
[2023-10-13 18:39:01,193] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,195] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,196] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,197] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,198] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(
[2023-10-13 18:39:01,199] sopel.bot            WARNING  - Deprecated since 7.1, will be removed in 9.0: SopelWrapper will soon be replaced with contextvars. See https://github.com/sopel-irc/sopel/issues/2460
  File "/home/dgw/github/sopel-irc/sopel/sopel/bot.py", line 869, in dispatch
    wrapper = SopelWrapper(

Even better, the @lifecycle.deprecated decorator doesn't appear in documentation anywhere; Sphinx doesn't know about it. That's why we also have to add .. deprecated:: blocks to the relevant docstrings when we mark stuff. (In short, docstring warns against future use while the decorator warns about existing use.)

Finally, the above is moot anyway right now because decorating class SopelWrapper makes it disappear from the documentation. The decorator wraps the class in an undocumented function, which is skipped when Sphinx scans the module for members to document. (:undoc-members: isn't included in the autodoc directives for package/bot.)

The deprecated decorator would need to be used on SopelWrapper.__init__(), not the class itself. See how the old sopel.tools.OutputRedirect type is marked:

sopel/sopel/tools/__init__.py

Lines 115 to 127 in ec9691b

class OutputRedirect:
"""Redirect the output to the terminal and a log file.
A simplified object used to write to both the terminal and a log file.
"""
@deprecated(
"Remnant of Sopel's pre-7.0 logging system. "
"You shouldn't have been using this anyway.",
version='8.0',
removed_in='8.1',
)
def __init__(self, logpath, stderr=False, quiet=False):

(This example also demonstrates that warning_in needn't be specified if the desired value is equal to version.)

dgw
dgw approved these changes Oct 14, 2023
View reviewed changes
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Checked documentation preview and 👍

image

Anyone who goes "oh shit!" will look at the issue, and it won't matter that the specific timeline isn't included here. I'm not even positive that we'll finish this transition before any specific version yet, so might as well leave it ambiguous.

@SnoopJ SnoopJ force-pushed the doc/warn-sopelwrapper-deprecated branch from ac884ea to 8060ab7 Compare October 14, 2023 04:17
@dgw dgw mentioned this pull request Jul 3, 2023
Open
4 tasks
@dgw dgw merged commit 981ceea into sopel-irc:master Oct 14, 2023
@SnoopJ SnoopJ deleted the doc/warn-sopelwrapper-deprecated branch October 14, 2023 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgw dgw dgw approved these changes

Assignees
No one assigned
Labels
Housekeeping Code cleanup, removal of deprecated stuff, etc. Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon.
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@SnoopJ @dgw