Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Update configuration from Manager server #118

Closed
wants to merge 1 commit into from
Closed

[WIP] Update configuration from Manager server #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ file is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and
this project adheres to [Semantic
Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Changed

- Replaced `reload_config` request handler with `update_config`.

## [0.5.0] - 2024-11-26

### Changed
Expand Down Expand Up @@ -135,6 +141,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
- Send the generated `time series` to `giganto`'s ingest.
- Save the model's id and the last time the timeseries was sent to a file.

[Unreleased]: https://github.com/aicers/crusher/compare/0.5.0...main
[0.5.0]: https://github.com/aicers/crusher/compare/0.4.1...0.5.0
[0.4.1]: https://github.com/aicers/crusher/compare/0.4.0...0.4.1
[0.4.0]: https://github.com/aicers/crusher/compare/0.3.2...0.4.0
Expand Down
4 changes: 2 additions & 2 deletions src/request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,8 @@ impl review_protocol::request::Handler for RequestHandler {
Ok(())
}

async fn reload_config(&mut self) -> Result<(), String> {
info!("start reloading configuration");
async fn update_config(&mut self) -> Result<(), String> {
info!("Updating configuration");
Copy link
Contributor

Choose a reason for hiding this comment

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

@sophie-cluml and @danbi2990,
Does calling this method guarantee that the update process will start? If not, we need to clarify that this method only attempts or plans to update the configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sophie and I discussed that the update process will be implemented in the next issue.
The next issue will proceed after the same functionality is completed for the Semi-supervised Engine.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get what you said. Is this going to wait for the next issue being completed? Or, this message will be handled later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The latter is right. It will be handled later because it needs to fetch the configuration from the manager server.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this PR is temporarily being halted, it would be better to mention it to let others know of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original plan was to merge this PR first and then implement the next issue.

I thought it was fine because this PR only changes the method name while maintaining the same behavior as before.

However, as you suggested, it seems reasonable to wait until the next issue is completed. For now, I will mark this PR as [WIP]. Thank you!

self.config_reload.notify_one();
Ok(())
}
Expand Down