WIP: Revamp config management #1104
Draft
Conversation
This file contains 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
Make it more explicit that this is just a wrapper to external libraries so users should read their documentation in full and which functions are used exactly. Signed-off-by: Leandro Lucarella <[email protected]>
`marshmallow_dataclass.dataclass` is intended to be used only when using `my_dataclass.Schema` to get the schema. But using this is not very convenient when using type hints as they are not well supported by `marshmallow`, as the `load()` function can't have hints. This is actually why `load_config()` exists in the first place, so we are using `class_schema()` instead, so we don't really need that our types are decorated with `marshmallow_dataclass`, we can use the built-in `dataclass` instead, we just need to add the appropriate metadata if we want more complex validation. Using `class_shema()` is also necessary to be able to pass a `base_schema`, which we'll need when we want to use schemas with custom fields, like the ones provided by `frequenz.quantities`. Because of this, we just drop support for `marshmallow_dataclass.dataclass` and we'll require that built-in dataclasses are used in the future. Signed-off-by: Leandro Lucarella <[email protected]>
This is necessary to use custom fields when loading configurations. Signed-off-by: Leandro Lucarella <[email protected]>
This is just the last step to be able to enforce via the type system that only built-in dataclasses are used. Signed-off-by: Leandro Lucarella <[email protected]>
As this package gets more stuff, the old name used when the actor was stored in the `frequenz.actor` package doesn't make much sense and is confusing. Signed-off-by: Leandro Lucarella <[email protected]>
This is to follow the naming convention of actors and make it more obvious that it is an actor. Signed-off-by: Leandro Lucarella <[email protected]>
`pylint` is much more flaky than `mypy`, so better just have `mypy` doing those checks. Signed-off-by: Leandro Lucarella <[email protected]>
github-actions
bot
added
part:docs
Comment on lines
+121
to
+134
| @overload | ||
| async def new_receiver( | ||
| self, | ||
| *, | ||
| limit: int = 10, | ||
| wait_for_first: bool = True, | ||
| ) -> Receiver[Mapping[str, Any]]: ... | ||
|
|
||
| @overload | ||
| async def new_receiver( # pylint: disable=too-many-arguments | ||
| self, | ||
| *, | ||
| limit: int = 10, | ||
| wait_for_first: bool = True, |
There was a problem hiding this comment.
I think the most common use case is when limit=1, not 10.
In most scenarios we need only one latest config.
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.
No description provided.