ynput / ayon-aquarium

AYON addon for Aquarium integration
https://ayon.ynput.io/features?addons=aquarium
Apache License 2.0
1 stars 1 forks source link

Only subscribe to topics in the ALLOWED_AQ_TOPICS and not in IGNORED_AQ_TOPICS #6

Closed ymoriaud closed 4 months ago

ymoriaud commented 5 months ago

The PR #2 added a list of allowed and ignored events' topic, to avoid unnecessary process. To limit the number of events that the leecher have to process, we can reuse the same list here:

https://github.com/ynput/ayon-aquarium/blob/ea0e891187e601675408219b7ae54157701666b3/services/leecher/leecher/leecher.py#L69

by something like that:

ignored_set = set(IGNORED_AQ_TOPICS)
allowed_set = set(ALLOWED_AQ_TOPICS)

allowed_topics = allowed_set - ignored_set

for topic in allowed_topics:
    _AQS.listener.subscribe(topic, callback)
tweak-wtf commented 5 months ago

in general... would you rather limit the events that the leecher receives from aquarium or the evnts it dispatches?

ymoriaud commented 5 months ago

Both are fine. The optimisation is minor, since topics are filtered in the callback.

But the callback function will grow based on #7, and to keep it readable and to split responsibilities, moving the "allowed_topics" outside the callback function will allow to only use the callback for checking the event context and sending the event into Ayon's database. Instead of having a big callback function doing a lots of stuff.

Does that seem logic to you ?

tweak-wtf commented 5 months ago

i think it's good. using main() for configuring the allowed event type coming from aquarium using callback() for filtering if the events belongs to an "ayon-synced" project

tweak-wtf commented 5 months ago

this is implemented as part of https://github.com/ynput/ayon-aquarium/pull/8

ymoriaud commented 4 months ago

Closed by #8