-
Notifications
You must be signed in to change notification settings - Fork 18
Replace DatabaseWrapper.__getattr__() with a simpler database attribute #287
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
Replace DatabaseWrapper.__getattr__() with a simpler database attribute #287
Conversation
I contemplated removing the self.database assignment from DatabaseWrapper.init_connection_state() which would have broken this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Non-blocking questions
connection_created.connect(receiver) | ||
connection.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also assert that data is empty after calling close()
or is that not necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly could, but if close()
actually created a connection (thus firing the connection_created
signal) instead of closing, I think there would be other rather obvious problems.
return getattr(self, attr) | ||
raise AttributeError(attr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we now won't raise an error if something other than database
is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default implementation of __getattr__()
raises AttributeError
.
No description provided.