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

[fix] #4063: update configuration endpoints #4076

Closed

Conversation

0x009922
Copy link
Contributor

@0x009922 0x009922 commented Nov 23, 2023

Description

I've implemented the changes according to the linked issue:

  • Remove documentation retrieval from the API
  • Normalize DTO structure with the configuration file

Linked issue

Closes #4063

Benefits

A little step closer to implementing the Configuration RFC (#2585).

Checklist

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922 0x009922 added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Nov 23, 2023
client/src/client.rs Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Nov 23, 2023

Pull Request Test Coverage Report for Build 6975480014

  • 23 of 54 (42.59%) changed or added relevant lines in 3 files are covered.
  • 7001 unchanged lines in 128 files lost coverage.
  • Overall coverage decreased (-3.1%) to 56.338%

Changes Missing Coverage Covered Lines Changed/Added Lines %
client/src/client.rs 0 5 0.0%
cli/src/torii/routing.rs 0 10 0.0%
config/src/client_api.rs 23 39 58.97%
Files with Coverage Reduction New Missed Lines %
config/base/derive/src/view.rs 1 99.37%
config/src/block_sync.rs 1 95.0%
config/src/network.rs 1 93.75%
config/src/torii.rs 1 95.45%
config/src/wasm.rs 1 87.5%
core/src/smartcontracts/isi/block.rs 1 87.5%
config/src/kura.rs 2 79.41%
ffi/src/option.rs 2 71.43%
config/src/genesis.rs 3 72.92%
data_model/derive/src/has_origin.rs 3 95.16%
Totals Coverage Status
Change from base Build 5423219773: -3.1%
Covered Lines: 23040
Relevant Lines: 40896

💛 - Coveralls

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
config/src/client_api.rs Outdated Show resolved Hide resolved

/// Subset of [`super::iroha`] configuration.
#[derive(Debug, Serialize, Deserialize, Clone, Copy)]
pub struct ConfigurationDTO {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe call it PartialConfiguration or DynamicConfiguration?

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 thought that DTO suffix emphasizes the purpose to use this structure to interact with the client side.

Partial seems to be too general here (like Proxys we have already), while Dynamic might semantically fit.

@mversic
Copy link
Contributor

mversic commented Nov 23, 2023

I think returning Docs was rather silly. Thanks Dima

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922 0x009922 marked this pull request as draft November 27, 2023 06:20
@0x009922
Copy link
Contributor Author

The logger & dynamic reloads architecture needs to be refactored first.

@0x009922
Copy link
Contributor Author

@0x009922 0x009922 closed this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants