Don't copy monitoring address/port parameters into the DFK. #3522
Conversation
Prior to this PR, monitoring hub address and ZMQ port were stored as attributes of the DFK. The address also existed as an attribute on dfk.monitoring, and the ZMQ port was returned by dfk.monitoring.start Afte this PR, those values are not added to the DFK, but instead are accessed via dfk.monitoring. These two attributes are now only set on a new executor when monitoring is enabled, rather than always being intialised by the DFK. Default values now come from the executor __init__ method, which is a more usual style in Python for providing default values. See PR #3361 This is part of ongoing work to introduce more pluggable monitoring network connectivity - see PR #3315
| executor.hub_address = self.monitoring.hub_address | ||
| executor.hub_zmq_port = self.monitoring.hub_zmq_port | ||
| executor.monitoring_radio = self.monitoring.radio |
There was a problem hiding this comment.
I'm wondering if it would be better to simply attach the monitor to the executor, rather than copying these details over piecemeal. I haven't thought that through though, I just don't like the smell of multiple copies of the same data lying around.
There was a problem hiding this comment.
the whole model of populating state post-construction ad-hoc into various objects makes me generally uncomfortable in Parsl - way longer term I'd love some stronger notion of "existing in a context" that doesn't look like this. (absolutely not suggesting this as an approach but compare how pytest uses fixtures)
that's not for this PR
There was a problem hiding this comment.
although monitoring specific, this is part of the monitoring radio plugin prototype which I hope to reduce the context that needs to be passed around to be "here's the object you use to send your messages, it encapsulates everything needed for that".
Prior to this PR, monitoring hub address and ZMQ port were stored as attributes of the DFK. The address also existed as an attribute on dfk.monitoring, and the ZMQ port was returned by dfk.monitoring.start
Afte this PR, those values are not added to the DFK, but instead are accessed via dfk.monitoring.
These two attributes are now only set on a new executor when monitoring is enabled, rather than always being intialised by the DFK. Default values now come from the executor init method, which is a more usual style in Python for providing default values. See PR #3361
This is part of ongoing work to introduce more pluggable monitoring network connectivity - see PR #3315
Changed Behaviour
none
Type of change