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.

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

Change struct Configuration to enum Configs #59

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

sophie-cluml
Copy link
Contributor

@sophie-cluml sophie-cluml commented Feb 20, 2024

closes #58

@BLYKIM @dayeon5470 please help double-check if each module's Configs are okay.

@msk msk requested a review from minshao February 20, 2024 18:22
@dayeon5470
Copy link

dayeon5470 commented Feb 21, 2024

@sophie-cluml hog giganto_address 추가해주세요

@BLYKIM
Copy link
Contributor

BLYKIM commented Feb 21, 2024

이미 crusher에 적용되어있는 수정 가능한 configuration은 기존과 같이 giganto의 ingest_address, publish_address가 모두 필요합니다만,
crusher가 review에서 바라보는 node에 적용되는 시점에 수정할 것인지에 대해서는 판단을 맡기겠습니다.

@sophie-cluml sophie-cluml force-pushed the sophie/enum-configs branch 2 times, most recently from f69698a to ee73ab6 Compare February 21, 2024 02:13
@sophie-cluml
Copy link
Contributor Author

@BLYKIM I added CrusherConfig. Please check.

src/request.rs Outdated
#[derive(Debug, Deserialize, Serialize)]
pub struct CrusherConfig {
pub review_address: SocketAddr,
pub gignato_ingest_address: SocketAddr,
Copy link
Contributor

@BLYKIM BLYKIM Feb 21, 2024

Choose a reason for hiding this comment

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

gignato -> giganto , Option<SocketAddr>로 변경 부탁드려요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

CHANGELOG.md Outdated
@@ -248,6 +255,7 @@ without relying on the content of the response.

- `send_frame` and `recv_frame` to send and receive length-delimited frames.

[Unreleased]: https://github.com/petabi/oinq/compare/0.9.1...main
Copy link
Contributor

Choose a reason for hiding this comment

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

0.9.3...main으로 변경해주세요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, done!

CHANGELOG.md Outdated

### Changed

- Change struct `Configuration` to enum `Configs` and define configuration by
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change this to past tense please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done!

src/request.rs Outdated
pub giganto_name: Option<String>,
pub dump_file: Option<String>,
pub dump_size: Option<usize>,
pub enum Configs {
Copy link
Contributor

Choose a reason for hiding this comment

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

Config would be more appropriate, as enum could only be one of the Config kind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@minshao minshao merged commit 350a0ab into petabi:main Feb 22, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC - Update Configuration to support new configuration fields
4 participants