Skip to content
This repository was archived by the owner on Jul 28, 2020. It is now read-only.

Conversation

@MrYawe
Copy link
Contributor

@MrYawe MrYawe commented Aug 16, 2019

Example

launch_cron.js

require("./client");

const CronWorkflow = require("./Workflows/CronWorkflow");

await new CronWorkflow().schedule("* * * * *")

Same thing for tasks.

@MrYawe MrYawe requested review from jalric and pylebecq August 16, 2019 17:35
@MrYawe MrYawe requested a review from jalric August 16, 2019 17:39
@geomagilles
Copy link
Contributor

geomagilles commented Aug 17, 2019

@MrYawe let's talk about the output, I'm not sure to understand it fully.

  • id: what's this?
  • intentId: this should not the exposed naming (scheduleId)
  • targetType: why not a type in target? (and btw always use lowercase 'workflow' - we have to be consistent)
  • why a name?
  • it seems to me that canonicalName should be "CronWorkflow"
  • a date is probably missing "scheduledAt"

@MrYawe
Copy link
Contributor Author

MrYawe commented Aug 19, 2019

@geomagilles

id: what's this?

it's the id of the resource we just created (a schedule)

intentId: this should not the exposed naming (scheduleId)

I'm not sure to understand but today all requests made by the lib to the agent have an intent_id. I think we should do the same between the lib and alfred. You just want to "hide" this field to the client ?

targetType: why not a type in target? (and btw always use lowercase 'workflow' - we have to be consistent)

👍

why a name?

I think it may be useful to be able to choose the name of a schedule as an option (if you have multiple schedule for the same workflow for example). We can't pass this option from the lib today (we will wait for the new syntax ?) but this option is available and ready server side. Today the name of a schedule is an optional parameter and by default it's the name of the target (workflow or task).

it seems to me that canonicalName should be "CronWorkflow"

👍

a date is probably missing "scheduledAt"

yes. I will add the inserted_at and updated_at in the GraphQL response.

Copy link
Contributor

@pylebecq pylebecq left a comment

Choose a reason for hiding this comment

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

Seems good to me.

The only question I have is why do you uppercase the programming language?

@MrYawe
Copy link
Contributor Author

MrYawe commented Aug 19, 2019

@pylebecq
Because programming_language is a GraphQL enum type so it must be an uppercased string.

@MrYawe MrYawe force-pushed the scheduling_alfred branch from 4fb88e0 to 33ba7e5 Compare August 19, 2019 15:58
@MrYawe MrYawe force-pushed the scheduling_alfred branch from 999f825 to 33ba446 Compare August 26, 2019 09:22
@MrYawe MrYawe force-pushed the scheduling_alfred branch from 33ba446 to 0b85c2f Compare August 26, 2019 09:41
@MrYawe MrYawe merged commit dc3f0c2 into master Aug 26, 2019
@MrYawe MrYawe deleted the scheduling_alfred branch August 26, 2019 09:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants