Relay Subscribe #5
Conversation
… into tests/relay-subscribe
fbarbu15
requested review from
AlejandroCabeza,
AlbertoSoutullo and
romanzac
| # The retry decorator is applied to handle transient errors gracefully. This is particularly | ||
| # useful when running tests in parallel, where occasional network-related errors such as | ||
| # connection drops, timeouts, or temporary unavailability of a service can occur. Retrying | ||
| # ensures that such intermittent issues don't cause the tests to fail outright. | ||
| @retry(stop=stop_after_delay(0.5), wait=wait_fixed(0.1), reraise=True) |
There was a problem hiding this comment.
Just out of curiosity, why is this no longer needed?
There was a problem hiding this comment.
I realized it makes more sense to handle retries case by case at higher level, instead of having all HTTP requests that fail retried by default.
There are more and more test cases where it is expected to have an error and I wanted to avoid retries for those cases.
So removing this retry gives the tests more control
tests/relay/test_subscribe.py
Outdated
| for pubsub_topic in VALID_PUBSUB_TOPICS: | ||
| self.check_publish_without_subscription(pubsub_topic) | ||
|
|
||
| def test_unsubscribe_from_sume_pubsub_topics(self): |
There was a problem hiding this comment.
Typo, I guess this is "same"?
There was a problem hiding this comment.
Yes, will fix, thanks :)
| if self.node1.is_nwaku(): | ||
| pass # nwaku doesn't fail in this case |
There was a problem hiding this comment.
Am I right when I say that you mentioned you were gathering all differences between Waku implementations? Where does that live?
There was a problem hiding this comment.
Most of them as github issues/bugs if they are serious enough.
But now that you mentioned it, maybe it makes sense to start a notion page for everything that I find. Because for this one in particular I didn't raised a bug because doesn't look that serious, it's more of an edge case. But either way it should be documented
Will start a notion page, thanks for the hint :)
There was a problem hiding this comment.
Here's the page I started, it's not much but still better than nothing :)
There was a problem hiding this comment.
Aside from @AlbertoSoutullo's comments I got nothing to add :)
|
@fbarbu15 I am learning from each of your PRs. I am curious about this sequence: I haven't seen retry in ensure_ready. Is the ensure_ready() still valuable to run, when we could fail in the next step as well? What if container restarted, but API not ready yet ? Just curious. |
It is decorated with the tenacity retry decorator |
duplicate validator for topicerror when trying to re-subscribe to previously unsubscribed topic go-waku#922 and bug: discv5/discover.go messages flooding the docker DEBUG logs go-waku#923) in the process