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

[dpy2_unstable] Updates the Red-Lavalink dependency #5628

Closed
wants to merge 40 commits into from

Conversation

Crossedfall
Copy link
Contributor

Description of the changes

Closing Cog-Creators/Red-Lavalink#99 broke fresh installs for the dpy2 branch. Updating to the superseding PR ( Cog-Creators/Red-Lavalink#122 ) allows pip to install from this branch again.

Collecting Red-DiscordBot
  Cloning https://github.com/Cog-Creators/Red-DiscordBot (to revision V3/dpy2_unstable) to c:\users\xxxx\appdata\local\temp\pip-install-xzgzwvqr\red-discordbot_1a7eefba8fc8420788779cc0795d8ca3
  Running command git clone --filter=blob:none --quiet https://github.com/Cog-Creators/Red-DiscordBot 'C:\Users\xxxx\AppData\Local\Temp\pip-install-xzgzwvqr\red-discordbot_1a7eefba8fc8420788779cc0795d8ca3'
  Running command git checkout -b V3/dpy2_unstable --track origin/V3/dpy2_unstable
  Branch 'V3/dpy2_unstable' set up to track remote branch 'V3/dpy2_unstable' from 'origin'.
  Switched to a new branch 'V3/dpy2_unstable'
  Resolved https://github.com/Cog-Creators/Red-DiscordBot to commit 6f3067841cb120e3ee791c086882d54dc96f1da0
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting Red-Lavalink@ git+https://github.com/Cog-Creators/Red-Lavalink@refs/pull/99/merge#egg=Red-Lavalink
  Cloning https://github.com/Cog-Creators/Red-Lavalink (to revision refs/pull/99/merge) to c:\users\xxxx\appdata\local\temp\pip-install-xzgzwvqr\red-lavalink_208901d112a04800afb89f28b90494fc
  Running command git clone --filter=blob:none --quiet https://github.com/Cog-Creators/Red-Lavalink 'C:\Users\xxxx\AppData\Local\Temp\pip-install-xzgzwvqr\red-lavalink_208901d112a04800afb89f28b90494fc'
  WARNING: Did not find branch or tag 'refs/pull/99/merge', assuming revision or ref.
  Running command git fetch -q https://github.com/Cog-Creators/Red-Lavalink refs/pull/99/merge
  fatal: couldn't find remote ref refs/pull/99/merge
  error: subprocess-exited-with-error

Have the changes in this PR been tested?

Tested to ensure it installs, I do not know if it breaks anything past that.

PS D:\GitHub\crossed-cogs> pip install -U --force-reinstall git+https://github.com/crossedfall/Red-DiscordBot@patch-1#egg=Red-DiscordBot
[--SNIP--]
Collecting Red-Lavalink@ git+https://github.com/Cog-Creators/Red-Lavalink@refs/pull/122/merge#egg=Red-Lavalink
  Cloning https://github.com/Cog-Creators/Red-Lavalink (to revision refs/pull/122/merge) to c:\users\xxxx\appdata\local\temp\pip-install-olig_o4a\red-lavalink_3f86df822f9243e4b9d503136a471acd
  Running command git clone --filter=blob:none --quiet https://github.com/Cog-Creators/Red-Lavalink 'C:\Users\xxxx\AppData\Local\Temp\pip-install-olig_o4a\red-lavalink_3f86df822f9243e4b9d503136a471acd'
  WARNING: Did not find branch or tag 'refs/pull/122/merge', assuming revision or ref.
  Running command git fetch -q https://github.com/Cog-Creators/Red-Lavalink refs/pull/122/merge
  Running command git checkout -q f61fa805741847f689fe4540e956edf47d2ecb08
  Resolved https://github.com/Cog-Creators/Red-Lavalink to commit f61fa805741847f689fe4540e956edf47d2ecb08
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
[--SNIP--]
Successfully installed Babel-2.9.1 Markdown-3.3.4 PyNaCl-1.4.0 PyYAML-5.4.1 Pygments-2.10.0 Red-Commons-1.0.0 Red-DiscordBot-3.5.0.dev1 Red-Lavalink-0.10.0 aiohttp-3.7.4.post0 aiohttp-json-rpc-0.13.3 aiosqlite-0.17.0 appdirs-1.4.4 apsw-wheels-3.36.0.post1 async-timeout-3.0.1 attrs-21.2.0 cffi-1.14.6 chardet-4.0.0 click-8.0.1 colorama-0.4.4 commonmark-0.9.1 contextlib2-21.6.0 discord.py-2.0.0a3741+ga909c1f fuzzywuzzy-0.18.0 idna-3.2 
multidict-5.1.0 psutil-5.8.0 pycparser-2.20 python-Levenshtein-wheels-0.13.2 python-dateutil-2.8.2 pytz-2021.1 rich-10.9.0 schema-0.7.4 six-1.16.0 typing-extensions-3.10.0.2 yarl-1.6.3

A lot of this is removing `.replace(...)` which while not technically
needed, simplifies the code base. There's only a few changes that are
actually necessary here.
Here's the list of the changes that have been made here:
- Documented that `{channel}` in CCs might be a thread
- Prevented usage of threads in `[p]announceset channel`
- Allowed usage of threads in `[p](un)ignore channel`
    - Also, threads inherit parent's and parent's category ignore lists
- Made `[p](un)mutechannel` refer to parent in threads
- Made permissions system apply rules of (only) parent in threads
- Allowed (documented) usage of threads with `Config.channel()`
    - Separate thread scope is still in the picture though it might be
      better to do this in separate PR?
- Prevented settings filter list for threads in Filter cog
- Updated Filter cog to inherit parent channel's filter list in threads
- Prevented settings rules for threads with `[p]permissions`
    - Permissions cog does not perform any validation for IDs
      when setting through YAML so that has not been touched
- Prevented adding stream alerts for threads
- Fixed assertion error in `bot.message_eligible_as_command()`
- Added support for passing `Thread` to `bot.embed_requested()`
    - Threads don't have their own settings and inherit from parent
    - Prevented usage of threads in `[p]embedset channel`
- Prevented usage of threads in `[p]set ownernotifications`
- Prevented usage of threads in `[p]diagnoseissues` (more info below)
- Added support for passing `Thread` to `channel` when creating case
    - Added `parent_channel_id` attribute to Modlog API's Case
    - Updated case's content to show both thread and parent information
- Updated type hints

There are some known missing things:
- `[p]diagnoseissues` does not accept threads (I didn't feel like
  writing strings today :))
- Quite a bit of user-facing docstrings refer to current channel
  when they also mean thread (should we fix this?).
  - Developer-facing docs do too but I don't think that's an issue.
This means adding `override` keyword-only parameter and causing
small breakage by swapping RuntimeError with discord.ClientException.
Honestly, this was a rather bad implementation anyway...

Breaking but actually not really - it was provisional.
BTW, that should really be in core instead of what we have now...
This is breaking but it's best to resolve it like this.

Most (but not all) of the functionality of ConversionFailure
can be replicated with Context.current_parameter.

It is (currently) impossible to get the equivalent of
`ConversionFailure.argument` from d.py alone.
If this turns out to be useful, we might try to implement on our own
though it would be preferred to have this directly in d.py.
@Drapersniper
Copy link
Contributor

For it to work as intended you would also need to update the whole of audio including #5593.

when #5593 is merged - and I resolve the conflicts ill give jack a branch for him to merge into this

@Crossedfall
Copy link
Contributor Author

Mind that developers are currently unable to install Redbot from the dev branch to prepare for the coming April 30th changes. So maybe it would be better to drop the lavalink dependency down to the current release instead? At least until the changes in #5593 are finalized? Unless you think it's going to be merged really soon.

@Drapersniper
Copy link
Contributor

Mind that developers are currently unable to install Redbot from the dev branch to prepare for the coming April 30th changes. So maybe it would be better to drop the lavalink dependency down to the current release instead? At least until the changes in #5593 are finalized? Unless you think it's going to be merged really soon.

was due to be merged yesterday - but Jack raised some concerns and blocked the merge so right now its 🤷 - a quick and easy fix would be reopening the old PR so that you all can install from it.

@Jackenmen
Copy link
Member

Should be resolved now.

@Jackenmen Jackenmen closed this Mar 21, 2022
@Crossedfall Crossedfall deleted the patch-1 branch March 21, 2022 15:38
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.

4 participants