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

ADR-05 test execution #18

Draft
wants to merge 8 commits into
base: dev
Choose a base branch
from
Draft

ADR-05 test execution #18

wants to merge 8 commits into from

Conversation

andrey-kuprianov
Copy link
Contributor

@andrey-kuprianov andrey-kuprianov commented Jul 18, 2022

Closes #13.

The rendered document.

@andrey-kuprianov andrey-kuprianov self-assigned this Jul 18, 2022
@andrey-kuprianov andrey-kuprianov added this to the Atomkraft prototype milestone Jul 18, 2022
@andrey-kuprianov andrey-kuprianov requested a review from a team July 18, 2022 07:55
## React (Generate)

From `React` component it needs one function provided via an agreed upon entry point:
- `get_trace_reactor(trace, reactor = None)` provides the reactor for the given trace, and checks that they match each other.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to clarify this part: isn't the return value of this function a path to the reactor file (this, at least, makes the most sense to me). In that case, giving a reactor value as an argument doesn't make sense.

I'd like to suggest that we explicitly ask for checking a reactor by invoking check_reactor function and that the execute module reads from the conf file about the default reactor path if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point! the formulation is not precise enough, let me refine it... Better like that?

  • get_trace_reactor(trace, reactor = None) takes the optional path to the reactor as argument, and returns the path to the reactor file if it matches the provided trace, or raises an exception if it doesn't. If reactor argument is omitted, the default reactor should be used.

- `get_trace(trace = None)` retrieves the ITF trace, either the default one, or from the provided location;
- `get_model_trace(model = None, config = None, sample = None)` provides a trace from the model, using either default, or the provided parameters.

## React (Generate)
Copy link
Collaborator

Choose a reason for hiding this comment

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

React calls different associations than Reactor (I would guess most people would connect it somehow to reactive programming. I suggest the name ReactorRoom (as in my PR) or to settle on plain Generate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, you are right; I forgot about that other React...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, I like the name Reactor. Short and clear enough. I just caught myself at calling your component that way in my mind:)

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.

2 participants