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

Flatten round configuration and consolidate related components #639

Merged
merged 1 commit into from
Nov 11, 2019

Conversation

aklenik
Copy link
Contributor

@aklenik aklenik commented Nov 5, 2019

Resolves #636

The PR includes:

  • Converting caliper-flow to a class, and renaming it to CaliperEngine
  • Flattening the round configuration syntax, and moving the processing to DefaultTest, which is renamed to RoundOrchestrator
  • Move the round-related objects (monitor, report, etc.) to RoundOrchestrator, making CaliperEngine more lightweight (only flow control).
  • Instead of the modules loading the configuration objects themselves from a file, they should receive the already loaded object. This will make the testing easier (no more fake YAML loader mocks).
  • Instead of passing around configuration file paths, the modules could query them from the Config module (including the paths passed to the workers).
  • Extract worker process message handling into a base class.

@aklenik
Copy link
Contributor Author

aklenik commented Nov 5, 2019

@nklincoln These are the changes I plan in this PR. I'll add commits gradually, so it's easier to follow the changes (I'll squash at the end). Plus it's easier to run every test through the CI, hence the draft PR :)

@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2019

This pull request introduces 2 alerts when merging b82a3e4 into b6cf1eb - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

@aklenik aklenik force-pushed the round-config-refactor branch 2 times, most recently from dd1d9d6 to 23c045e Compare November 6, 2019 20:54
@aklenik aklenik marked this pull request as ready for review November 6, 2019 23:07
@aklenik
Copy link
Contributor Author

aklenik commented Nov 6, 2019

@nklincoln The PR is ready for review. Let me know when it's time to squash

@nklincoln nklincoln self-assigned this Nov 11, 2019
@nklincoln nklincoln added the PR/under review The PR is currently under review label Nov 11, 2019
Copy link
Contributor

@nklincoln nklincoln left a comment

Choose a reason for hiding this comment

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

👌

Signed-off-by: Attila Klenik <a.klenik@gmail.com>
@aklenik aklenik merged commit 788f31c into hyperledger:master Nov 11, 2019
@aklenik aklenik deleted the round-config-refactor branch November 11, 2019 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/under review The PR is currently under review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flatten the round descriptions in the benchmark configuration
2 participants