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

2.0.2 update #117

Merged
merged 40 commits into from
Aug 28, 2022
Merged

2.0.2 update #117

merged 40 commits into from
Aug 28, 2022

Conversation

majordoobie
Copy link
Collaborator

Ready for review, please. Combined #113 #116 #112 #115

Changes

  • discord_links.py now supports await client.close()
  • client.py no longer handles the creation and management of asyncio loops. Instead, it will just handle cleaning itself and allow wrappers such as asyncio.run to do the cleaning.
  • Added discord_cog example to showcase how to create a bot that inherits from the discord.exe.Bot
  • Wrapped client.login in a context manager to handle situations where the login failed. This allows cleanup of the session created
  • Updated all examples to use the new async pattern
  • Removed the discord bots from running in the GitHub action testing. I tried to make them work but was recommended by both discord.py and disnake forums not to do the tests there
  • Extended the python versions that GitHub Actions uses to test the code. This also means that we dropped Python versions 3.5 and 3.6 as they do not support asyncio.run

majordoobie and others added 30 commits August 18, 2022 12:01
…ers to close the client connection from within a async function such as async main(). This is useful to allow users to use the `asyncio.run()` wrapper properly
…g to grab a dictionary of members in the clan but that kwards was not available.

I added
```py
    @cached_property("_cs_members_dict")
    def members_dict(self) -> typing.Dict[str, ClanMember]:
        """Dict[str, :class:`ClanMember`]: A dict of members that belong to the clan."""
        return {m.tag: m for m in self._iter_members
 ```

 To clans to have the attribute available for the player iterator
… the session created was only closed when there was a successful interaction with the API. If there is a failure with logging in, for example, the session would not be closed.

The fix used is wrapping the body with a session context manager to automatically cleanup after itself.
…t to properly invoke a command context and from the forums it is not recommended to do so
…asyncio.run wrapper (recommended by asycion) and added a cogs example
…tion methods that coc.EventsClient has isntead of the lists
Copy link
Owner

@mathsman5133 mathsman5133 left a comment

Choose a reason for hiding this comment

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

Great, thanks! Most of my comments are just musings, feel free to reply to as much or little as you want, although most of it looks good to go.

Should probably add to the changelog, too. It's in docs/misc/changelog I think

coc/client.py Outdated
except Exception as error:
LOG.error("Invalid credentials\n%s", error)
await http.close()
raise
Copy link
Owner

Choose a reason for hiding this comment

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

Does this re-raise the InvalidCredentials error? For people who don't have logging setup a detailed exception is important because it's ambiguous why it failed otherwise

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it re-raises coc.InvalidCredentials so the user should already be tracking. You can see it in action in the examples

coc/client.py Outdated Show resolved Hide resolved
self.dispatch("on_client_close")
self.loop.run_until_complete(self.http.close())
self.loop.close()
async def close(self) -> None:
Copy link
Owner

Choose a reason for hiding this comment

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

Idk if anyone used it but should this still dispatch that on_client_close event?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried looking at the dispatch stuff but I don't really understand it. Could you provide some guidance on it?

coc/events.py Outdated Show resolved Hide resolved
coc/http.py Show resolved Hide resolved
examples/discord_links.py Outdated Show resolved Hide resolved
examples/events_example.py Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
.gitignore Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@mathsman5133
Copy link
Owner

setup.py in the main directory also needs the version tag changed to v2.1.0

@majordoobie majordoobie merged commit f8f414b into master Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants