-
Notifications
You must be signed in to change notification settings - Fork 5
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
refactor: change workflow to simulation and node to step #205
refactor: change workflow to simulation and node to step #205
Conversation
Pull Request Test Coverage Report for Build 408821341Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with the switch from Node
to Step
and Workflow
to Simulation
.
Submission vs Status vs Simulation
It seems that we don't really have a simulation
object but a create
and a status
object for simulations. As such I think we can get rid of the baked_recipe
object inside of the simulation create object.
I am not sure where to leave the recipe resolution logic (ie: queenbee or queenbee-pollination) as we are going for a pollination-centric approach. I think we could benefit from a call sooner rather than later to hash this out.
Simulations vs Workflows
In the distant and very theoretical future I can imagine people running recipes that aren't strictly simulations (ie: machine learning or different automation jobs) but this future is so distant that there is no point me dying on this word
hill 😄
queenbee/simulation/status.py
Outdated
class WorkflowStatus(BaseStatus): | ||
"""Workflow Status""" | ||
type: constr(regex='^WorkflowStatus$') = 'WorkflowStatus' | ||
class SimulationStatus(BaseStatus): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to include some optional recipe information here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added source url to BaseStatus
object which will provide these information. Its up to the client side to parse the source to get the desired information if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we leave the source
as an optional parameter? I think it might be difficult to resolve the source
for each step
inside of queenbee-argo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! I replied to your other comment. I will leave it to optional and we will see if you can find the Function
for each Step
or a Run
.
After our call it seems we are happy using the following terminology:
We have also agreed to include a |
This commit is a proposal for renaming a number of current objects. It also adds a new object for submitting simulations. The proposal is to change Workflow to Simulation and Node to Step/SimulationStep. With this structure: - A `Recipe` is a collection of `DAGs`. - A `DAG` is a collection of `tasks`. - A `Simulation` is a collection of `Steps`. Each `Step` is an executed `Task` with an status.
With the new design: * Job: corresponds the SimulationSubmission object * JobStatus: corresponds to the SimulationStatus object
18f6a06
to
7f895c6
Compare
@AntoineDao, I updated the PR to use Job instead of Workflow/Simulation. You probably want to check this commit (4632ed4) which doesn't include updating the docs folder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're good overall. I would just make the source
in the BaseStatus
object optional in case it is difficult to resolve for steps.
🎉 This PR is included in version 1.19.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This commit is a proposal for renaming a number of current objects. It also adds a new object for submitting simulations.
The proposal is to change Workflow to Simulation and Node to Step/SimulationStep. With this structure:
Recipe
is a collection ofDAGs
.DAG
is a collection oftasks
.Simulation
is a collection ofSteps
. EachStep
is an executedTask
with an status.