RLink Forwarding Issue #2080
RLink Forwarding Issue #2080
Conversation
- Added variable for repetitively accessed dictionary value - extended if statement to also check if "sub" is not None to see if the subscription has actually been made. - Added on_remote_leave to empty dictionary of forwarded subscriptions - extended if statement to also check if "reg" is not None to see if the registration has actually been made. - Made "create forwarding registration" log debug level to match the subscription one. This is expected functionality and there can be large amounts of registrations which would cause log spam.
|
alright, GH is confusing me:
... BUT: this PR above now also shows the CI as running even though the PRs are 2 different PRs. mmmh. my best bet is that the CI integration in GH only looks at the actual code changes, no matter on what PR those are carried. anyways;) we'll see what happens. I actually wanted to check the reasons for the failed CI steps in this PR to get work going again- |
oberstet
added
needs-investigation
rlinks
|
superseded by (but preserving history) #2090 |
crossbar/worker/rlink.py
Outdated
| @@ -162,17 +165,17 @@ def on_event(*args, **kwargs): | |||
| uri)) | |||
| return | |||
|
|
|||
| if sub_details["id"] not in self._subs: | |||
| if sub_id not in self._subs: | |||
| self.log.info("subscription already gone: {uri}", uri=sub_details['uri']) | |||
| yield sub.unregister() | |||
There was a problem hiding this comment.
Looks like copy/paste mistake. Subs don't have unregister() they have unsubscribe() which is called further down
There was a problem hiding this comment.
yes, indeed, that seems definitely wrong. good catch!
There was a problem hiding this comment.
sadly, mypy (which is pretty good at static code analysis) does not catch that:
(cpy311_7) oberstet@intel-nuci7:~/scm/crossbario/crossbar$ git log -n1
commit 507726e70eb329fda87b8f009b6310435dd85b31 (HEAD -> updating_dependencies, origin/updating_dependencies)
Author: Tobias Oberstein <tobias.oberstein@gmail.com>
Date: Wed Aug 23 19:15:11 2023 +0200
use latest release of eth-abi published 08.06.23
(cpy311_7) oberstet@intel-nuci7:~/scm/crossbario/crossbar$ python -V
Python 3.11.1
(cpy311_7) oberstet@intel-nuci7:~/scm/crossbario/crossbar$ tox -e mypy
mypy: commands[0]> mypy --exclude '(test_.*\.py)|(syntaxerror\.py)' --ignore-missing-imports crossbar
Success: no issues found in 215 source files
mypy: OK (0.33=setup[0.05]+cmd[0.28] seconds)
congratulations :) (0.40 seconds)
(cpy311_7) oberstet@intel-nuci7:~/scm/crossbario/crossbar$
There was a problem hiding this comment.
It is, sorry. Good spot.
Now that this PR has been closed and re-opened because of CI problems, I can't make the changes. Is there anything I can do to help?
There was a problem hiding this comment.
sorry for the reopening mess .. I was aware that it'll be more mess .. but unfort., this is a good example why 1 tiny issue (logs deleted after N days by GH, and so on) leads to more and more things ... which is why all of this is such a time burner
in general:
- on the one hand, we need to resolve Move CI fully to hosted GH runners again #2092 to make the hosted CI work again. rip out "self-hosted", replace with GH runners, .. that is, work on GH actions workflow files
- until 1. is fixed, you can also work on CI with local testing
I just tried tox -e sphinx,flake8,mypy,yapf,bandit,pytest. and other targets.
now comes the fun;) here is just one that gives another good example of "time burner":
sphinx fails currently. because it is set to "warnings are errors" (conservative). and
Warning, treated as error:
Failed to read intersphinx_mapping[https://docs.python.org/], ignored: SphinxWarning('The pre-Sphinx 1.0 \'intersphinx_mapping\' format is deprecated and will be removed in Sphinx 8. Update to the current format as described in the documentation. Hint: "intersphinx_mapping = {\'<name>\': (\'https://docs.python.org/\', None)}".https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html#confval-intersphinx_mapping')
intersphinx is for auto linking our docs to other popular projects' docs.
I don't know what intersphinx config shit changed, and I have zero motivation to find out;) but it needs fixing. I could go on. we've tried hard over the years to arrive at robust, working CI. I gave up. maybe I am just too exhausted, but this kind of stuff needs pretty much constant attention. why can't I write some CI today, make it work perfectly, and then rely on that it will continue to work next week when there are no changes on our side? I don't get it. 2 + 2 is 4 today, and it should be forever, but I can't rely on that tomorrow? wtf.
There was a problem hiding this comment.
forgot to mention: if you want, you can work locally or you can create new PRs or branch from mine or whatever .. I don't mind closing "mine" again .. whatever works works for me;)
There was a problem hiding this comment.
I had a read of the CI bug you linked (https://github.com/orgs/community/discussions/49242) and think this might work:
If you re-open this PR I can push the change suggested above and I believe it should re-trigger the CI to make new logs.
What do you think?
There was a problem hiding this comment.
it should re-trigger the CI to make new logs.
fwiw, I've reopened this one, but I don't think it'll help, since the self-hosted runner is gone (#2092 )
- Improved process of adding subscription to _subs and registration to _regs - sub needs to be unsubscribed, not unregistered
Fix for #2079