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

Refactor connector opsdroid pointer #725

Closed
jacobtomlinson opened this issue Nov 5, 2018 · 7 comments · Fixed by #749
Closed

Refactor connector opsdroid pointer #725

jacobtomlinson opened this issue Nov 5, 2018 · 7 comments · Fixed by #749

Comments

@jacobtomlinson
Copy link
Member

jacobtomlinson commented Nov 5, 2018

Currently in the Connector class we expect opsdroid to pass a pointer to itself on connect and disconnect. This seems a little pointless as the connector should be storing the pointer as an attribute anyway.

Steps

  • Add opsdroid as a kwarg in the __init__ method of the Connector class and store it as an attribute.
  • Remove the opsdroid kwarg from the connect and disconnect method as they are no longer needed.
  • All core connectors would also need updating to reflect this.
@zonagilreath
Copy link
Contributor

I will try to work on this.

@zonagilreath
Copy link
Contributor

This would be my first contribution to a project so I'm a little unsure of the protocol, should I also update the connector tests to reflect this change?

@FabioRosado
Copy link
Member

Hello @zonagilreath welcome to the project!
You are correct, the tests will need to be updated as well otherwise they will fail as they will have opsdroid as a param.

Let us know if you need any help 👍

@FabioRosado
Copy link
Member

FabioRosado commented Nov 6, 2018

Might be a good idea to add the opsdroid kwarg to the database class as well - this is related to the bug report of @colonelkrud

colonelkrud INFO opsdroid.core: Received stop signal, exiting. INFO opsdroid.core: Removing skills... 
INFO opsdroid.core: Stopping database ... 
ERROR opsdroid.core: Caught exception
ERROR opsdroid.core: {
'message': 'Task exception was never retrieved', 
'exception': TypeError('disconnect() takes 1 positional argument but 2 were given',), 
'future': <Task finished coro=<OpsDroid.handle_signal() done, defined at /usr/local/lib/python3.6/dist-packages/opsdroid/core.py:120>
 exception=TypeError('disconnect() takes 1 positional argument but 2 were given',)>}

@jacobtomlinson
Copy link
Member Author

Yes!

@zonagilreath
Copy link
Contributor

Sounds good, I'll checkout the db class once I get the connector tests fixed.

@zonagilreath
Copy link
Contributor

zonagilreath commented Nov 6, 2018

Oh also, since the issue said "Add opsdroid as a kwarg" I'm adding it as a keyword-only arg and it won't be possible to supply it positionally. Is this what is desired?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment