Improve project structure #15
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Leandro Lucarella <[email protected]>
We need to export also these symbols, as they are part of the `Dispatcher` interface: - DispatchEvent - Created - Updated - Deleted Signed-off-by: Leandro Lucarella <[email protected]>
The base class makes the documentation less clear and introduces a class hierarchy that could be error prone given that sub-classes are intended to be used in `match` statements. Signed-off-by: Leandro Lucarella <[email protected]>
These events are used by both the actor module and the top-level module, and we want to expose them publicly only in one place (the top-level module), so we don't confuse the users about where they should import symbols from. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
This makes it more clear that new receivers are created each time the method is called. Signed-off-by: Leandro Lucarella <[email protected]>
This is to follow the current actors naming convention. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
github-actions
bot
added
part:tests
Merged
llucax
added
cmd:skip-release-notes
|
Up for discussion. |
We use `lifecycle_events` and `ready_to_execute` respectively to make it more clear that the former provides events for any kind of dispatch changes, including creates and deletes, and that the later provides dispatches that are ready to be executed. Signed-off-by: Leandro Lucarella <[email protected]>
Split the example in 2, one for events and one for ready to execute processing, and add a match statement to make it more clear how it is intended to be used. Signed-off-by: Leandro Lucarella <[email protected]>
Marenz
reviewed
Mar 27, 2024
src/frequenz/dispatch/_dispatcher.py
Outdated
| dispatch_arrived = dispatcher.updated_dispatches.new_receiver() | ||
| dispatch_ready = dispatcher.ready_dispatches.new_receiver() | ||
| events = dispatcher.lifecycle_events.new_receiver() | ||
| ready = dispatcher.ready_to_execute.new_receiver() |
There was a problem hiding this comment.
I was not happy with the names, I like these more :)
Marenz
reviewed
Mar 27, 2024
|
|
||
| events_receiver = dispatcher.lifecycle_events.new_receiver() | ||
|
|
||
| async for event in events_receiver: |
There was a problem hiding this comment.
This makes me think we should also expose the client as dispatcher.client so the user can react to the events in a meaningful way when they want to update or create a new event as a response
There was a problem hiding this comment.
I don't get this. Why do you need the client?
There was a problem hiding this comment.
Well, anyone who wants to update, delete or create a request needs the client to do so, the high-level API
doesn't offer those things (and I don't see how it could do that better than the client)
Marenz
approved these changes
Mar 27, 2024
|
Great stuff |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dispatcherto its own module_DispatchEventBaseclassDispatcherexample in docstringDispatcherreturn receiver fetchersDispatchActortoDispatchingActorDispatcherexamplesupdated_dispatchesandready_dispatches