Correctly track session elements in dynamic build plan#2104
Correctly track session elements in dynamic build plan#2104abderrahim wants to merge 1 commit intomasterfrom
Conversation
When using a dynamic build plan, all elements are passed to the queue to be enqueued, but are only actually enqueued when they become required. This moves the tracking of session elements to the first queue rather than just take everything that we pass to the queue (which would be all elements). This is only used in the UI AFAICT, and doesn't actually affect the build.
abderrahim
requested review from
BenjaminSchubert,
cs-shadow,
gtristan and
juergbi
as code owners
| def _enqueue_plan(self, plan, *, queue=None): | ||
| queue = queue or self.queues[0] | ||
| queue.enqueue(plan) | ||
| self.session_elements += plan |
There was a problem hiding this comment.
The frontend still uses Stream.session_elements, which is now always empty.
I don't see how this branch can work as it is, but I could be missing something.
There was a problem hiding this comment.
We pass the list to the queue a bit earlier
queue.set_session_elements(self.session_elements)which then fills it as elements are getting enqueued.
Yeah, it's a bit confusing but I couldn't find a better way to do it.
There was a problem hiding this comment.
I guess the alternative is to make Element support more than one callback when it becomes required? This way both the Stream and the Queue can pass their own callbacks?
There was a problem hiding this comment.
Ah, it's just a reference to the list created in Stream. I think set_session_elements() threw me off as that doesn't actually set any elements, it just passes a reference to the list. With clearer naming and/or comment, it might be ok.
2 participants
When using a dynamic build plan, all elements are passed to the queue to be enqueued, but are only actually enqueued when they become required.
This moves the tracking of session elements to the first queue rather than just take everything that we pass to the queue (which would be all elements).
This is only used in the UI AFAICT, and doesn't actually affect the build.