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

add Workflow types to Service types #1049

Closed
wants to merge 1 commit into from

Conversation

ilgooz
Copy link
Contributor

@ilgooz ilgooz commented Jun 13, 2019

  • update service.Service type and definition.Service proto with Workflow type.
  • create type bindings for definition.Workflow proto to service.Service.Workflow.

depends on #1048

@ilgooz ilgooz force-pushed the feature/workflow-type-on-service branch from 366324a to bc2f0c2 Compare June 13, 2019 14:02
@ilgooz ilgooz requested a review from a team June 13, 2019 14:02
@ilgooz ilgooz force-pushed the feature/workflow-type-on-service branch from bc2f0c2 to 206dbb0 Compare June 13, 2019 15:02
* update service.Service type and definition.Service proto with Workflow type.
* create type bindings for definition.Workflow proto to service.Service.Workflow.
@ilgooz ilgooz force-pushed the feature/workflow-type-on-service branch from 206dbb0 to bf956c9 Compare June 13, 2019 15:20
@@ -56,6 +57,9 @@ type Service struct {
// Source is the hash id of service's source code on IPFS.
Source string `hash:"name:9"`

// Workflows keeps the workflows of service and is optional.
Workflows []workflow.Workflow `hash:"name:10"`
Copy link
Contributor

Choose a reason for hiding this comment

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

why service contains workflows? The workflow manages services so it should be another way around, or they should be next to each other.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw it, and I don't understand why workflows are one of the field in the service struct.

The workflow should be totally independent of the service. In fact, it doesn't need to know about service a all. As you wrote in workflow implementation it just a graph that get/recive data from the service. So as I see it, the workflow should be only focused on the flow of data. The other part will be ETL process from service into workflow. Otherwise again everything will be mixed together and tightly coupled which will lead to many problems (basic one is with tests):

  • in order to test workflow package service must be used
  • in order to load data into workflow the service must be used

And it could be done with simple unit tests, but it can't be so tightly coupled.

@krhubert krhubert closed this Jun 25, 2019
@mesg-bot
Copy link
Member

This pull request has been mentioned on MESG Community. There might be relevant details there:

https://forum.mesg.com/t/workflow-implementation/281/2

@NicolasMahe NicolasMahe deleted the feature/workflow-type-on-service branch September 5, 2019 04:27
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.

None yet

4 participants