-
Notifications
You must be signed in to change notification settings - Fork 43
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
WIP: Fix inits doing db IO #485
WIP: Fix inits doing db IO #485
Conversation
@devos50 can you review? (ignore the first commit, it will be dropped). |
self._connection_type = u"unknown" | ||
|
||
@inlineCallbacks | ||
def initlialize(self, communityclass=DebugCommunity, c_master_member=None, curve=u"low"): |
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.
There's a typo in the name of the method (should be initialize
).
@lfdversluis I gave some comments. Were this all occurrences of inits doing DB I/O btw? |
What my function found out yes. It turned out that message -> Implementation class was called all the time so many stack traces were gone by fixing that one. This one will be a headache to refactor though once it gets a Deferred, but that's for another PR. |
806e44d
to
468ac15
Compare
@whirm Ready for your review. |
2d8264b
to
8a317c9
Compare
impl = self.Implementation(self, authentication_impl, resolution_impl, distribution_impl, destination_impl, payload_impl, *args, **kargs) | ||
packet = "" | ||
if "packet" in kargs: | ||
packet = kargs["packet"] |
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.
you can do a kargs.get("packet", "")
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.
Ahh that will be much cleaner 👍
Yeah, it's not pretty but I guess it will have to do pending a major refactor... |
have you run the Tribler tests with this branch? |
@whirm No, I will do that after I fixed that line you commented on 👍 |
Tribler seems to run happy with this code, although one of the tests errored with a known error. |
retest this please |
Squash it! |
@whirm Since the container appears to be unstable, can I somehow run these changes + tribler on BBQ? I would feel more comfortable knowing that the tests pass then. Before a bug was introduced after all and the container hid it. |
@lfdversluis which container? yours or the test runner? You can always run your own jobs on jenkins. Just set up one on /pers/ |
retest this please |
3b8a8f7
to
4d512c3
Compare
@lfdversluis can you rebase this? |
4d512c3
to
e4249a5
Compare
__init__s no longer do DB IO (enhancement)
Fixes #484