This repository has been archived by the owner on Mar 3, 2023. It is now read-only.
Unvendor pyobj + fix py2->py3 compatability fixes. #3574
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
Code0x58
changed the title
Code0x58
force-pushed
the
py3-upgrade-issue
branch
from
89e112e to
f483c67
Compare
Comment on lines
+173
to
194
| def watch_topology(data, stats, event): | ||
| """ watch topology """ | ||
| if data: | ||
| topology = Topology() |
There was a problem hiding this comment.
I think I see the problem (mismatch number of passed arguments) you are trying to address.
"/Users/someUser/.pex/installed_wheels/7b3448a0635a4c114f98f11df06af06d250c297a/kazoo-2.7.0-py2.py3-none-any.whl/kazoo/recipe/watchers.py", line 162, in _log_func_exception
result = self._func(data, stat, event)
TypeError: watch_execution_state() takes 2 positional arguments but 3 were given
I'm not sure I follow the reasoning of adding event to each of the method signatures. If it is never used, why do we need to pass it at all?
There was a problem hiding this comment.
I think it helps by avoiding the questionable pattern here which can obscure the exceptions when they occur within the handler - at least we certainly won't see the TypeError now even if the parameter is junk (no more "exception occurred while handling exception", so that much easier to read)
There was a problem hiding this comment.
Ok. Thanks for clarifying. Makes sense to me.
huijunwu
approved these changes
Jul 20, 2020
|
Code0x58
force-pushed
the
py3-upgrade-issue
branch
from
f483c67 to
763c82a
Compare
|
thanks @thinker0, I just pushed a quick amendment that should resolve the |
|
@Code0x58 another |
Code0x58
force-pushed
the
py3-upgrade-issue
branch
from
763c82a to
cf3a3e6
Compare
|
thanks again @thinker0, I pushed another amendment. The amendment isn't 1-to-1 with the python2 implementation, but it's probably saner. I don't know if it will have a knock on effect down the line 🤞 |
|
@Code0x58 Another |
Code0x58
force-pushed
the
py3-upgrade-issue
branch
from
cf3a3e6 to
f23265f
Compare
|
humm, that error looks like it could be a deeper |
Code0x58
force-pushed
the
py3-upgrade-issue
branch
from
f23265f to
548635a
Compare
Code0x58
changed the title
Code0x58
force-pushed
the
py3-upgrade-issue
branch
2 times, most recently
from
1ec15dd to
e74fa85
Compare
|
CI passed on my machine, so hopefully the switch to the maintained version of |
|
@Code0x58 sorry. The last one goes like this. Other features seem to work fine. |
|
Thanks again @thinker0! So much for hopes of a dumb upgrade. I'll have to spend some time getting to understand the old and new |
This also uses the longer DataWatcher handler signature to avoid a TypeError, which it relies on to indicate it should retry with the shorter signature. Without this change, exceptions can be reported messily due to a raise in the except path being reasonably expected.
Code0x58
force-pushed
the
py3-upgrade-issue
branch
from
e74fa85 to
0cf3781
Compare
|
an initial look into |
|
+1 |
nicknezis
approved these changes
Jul 22, 2020
thinker0
approved these changes
Jul 22, 2020
nicknezis
pushed a commit
that referenced
this pull request
Sep 14, 2020
This also uses the longer DataWatcher handler signature to avoid a TypeError, which it relies on to indicate it should retry with the shorter signature. Without this change, exceptions can be reported messily due to a raise in the except path being reasonably expected.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Re. https://heronstreaming.slack.com/archives/C0VKQLKV0/p1595162763201800
This also uses the longer DataWatcher handler signature to avoid a TypeError, which it relies on to indicate it should retry with the shorter signature.
Also sorts #3575