Skip to content

Conversation

@llucax
Copy link
Contributor

@llucax llucax commented Oct 24, 2024

The ConfigManagingActor can now take more than one config file, and will read multiple files one after the other, overriding the previous values.

Fixes #65.

The file was moved from the `actor` package to the `config` package, but
the tests location wasn't updated.

This commit brings the test location in sync with the code location
again.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax requested a review from a team as a code owner October 24, 2024 11:08
@llucax llucax requested review from Marenz and removed request for a team October 24, 2024 11:08
@github-actions github-actions bot added part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:config Affects the configuration management labels Oct 24, 2024
@llucax llucax self-assigned this Oct 24, 2024
@llucax
Copy link
Contributor Author

llucax commented Oct 24, 2024

"""
await self.send_config()

parent_paths = {p.parent for p in self._config_paths}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unique_parent_paths = [..] for more clarity? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, it is a set; is it really necessary to say that items in a set are unique?

This comment was marked as off-topic.

@llucax llucax force-pushed the config-mutiple-files branch from ba0ce39 to 743bbaf Compare October 24, 2024 13:37
@llucax llucax force-pushed the config-mutiple-files branch from 743bbaf to 5cd4b40 Compare October 25, 2024 07:58
@llucax
Copy link
Contributor Author

llucax commented Oct 25, 2024

Enabled auto-merge.

@llucax llucax enabled auto-merge October 25, 2024 07:58
Copy link
Contributor

@daniel-zullo-frequenz daniel-zullo-frequenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a couple of minor comments to check for. And LGTM otherwise

The `ConfigManagingActor` can now take more than one config file, and
will read multiple files one after the other, overriding the previous
values.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax force-pushed the config-mutiple-files branch from c795bbf to c0f9fd5 Compare October 25, 2024 09:44
Copy link
Contributor

@daniel-zullo-frequenz daniel-zullo-frequenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'll wait for @Marenz or @cwasicki to approve it as they both had comments

@llucax llucax requested a review from Marenz October 25, 2024 10:59
@llucax llucax requested a review from cwasicki October 25, 2024 10:59
@llucax llucax added this pull request to the merge queue Oct 25, 2024
Merged via the queue into frequenz-floss:v1.x.x with commit ff786cd Oct 25, 2024
18 checks passed
@llucax llucax deleted the config-mutiple-files branch October 25, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:config Affects the configuration management part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests

Projects

Development

Successfully merging this pull request may close these issues.

Make ConfigManager read configuration from multiple files

4 participants