implement trigger methods #115
Conversation
alexei-led
commented
Jan 25, 2018
alexei-led
commented
- implement trigger methods
- extended version info: show server and trigger manager versions
| const getRoot = require('../root/get.cmd'); | ||
|
|
||
| const command = new Command({ | ||
| command: 'triggers <pipeline>', |
There was a problem hiding this comment.
Get operation should return a single trigger or an array of triggers.
Exactly as all other get operation were modeled.
- The argument passed can be a specific trigger id
- get triggers should return all triggers by default not related to a specific pipeline.
- passing a specific pipeline id as a filter should be an option
codefresh get triggers - get all triggers
codefresh get trigger TRIGGER_ID - get a specific trigger
codefresh get triggers -p PIPELINE_ID1 - get all triggers that match a specific pipeline
codefresh get triggers -p PIPELINE_ID1 -p PIPELINE_ID2 - get all triggers that match an array of pipelines
the pipeline should be an option.
There was a problem hiding this comment.
Currently, it's not required to get all triggers. Or get triggers for multiple pipelines. Will add this functionality when we will define relevant use cases in UI.
Triggers MVP: set/unset/list trigger(s) for selected pipeline (with minimal trigger type meta-data to create UI); get trigger details (mainly help text and links).
There was a problem hiding this comment.
i do think that getting all triggers is relevant.
it is not related to UI use-cases or not, it is a basic requirement to be able to get all.
the pipeline should be modeled as an option
There was a problem hiding this comment.
will add a TODO task to Jira to implement sooner or later
| }, | ||
| webDocs: { | ||
| category: 'Triggers', | ||
| title: 'Get triggers connected to the specified pipeline', |
There was a problem hiding this comment.
webDocs title is shown as a page title in the cli documention left navigation side menu.
it should be: 'Get Trigger'
There was a problem hiding this comment.
What is category for?
| }, | ||
| webDocs: { | ||
| category: 'Triggers', | ||
| title: 'Remove trigger from specified pipeline', |
There was a problem hiding this comment.
Context? Do you mean "Delete trigger" instead of "Remove trigger"?
| const deleteRoot = require('../root/delete.cmd'); | ||
|
|
||
| const command = new Command({ | ||
| command: 'trigger <event-uri> <pipeline>', |
There was a problem hiding this comment.
Delete should work with the id/name of the created resource
codefresh delete trigger TRIGGER_ID
There was a problem hiding this comment.
There is no unique ID. It's is a link between trigger-event and pipeline. It's identified by trigger-event URI (unique) and pipeline id (unique). I think there is no need to create an additional ID. It's the combination of both that is unique.
There was a problem hiding this comment.
same as in trigger-event, there should be a unique name that will be possible to get/delete by it.
you can also support the easy and nice way you provided but the trigger should have a unique name
| }, | ||
| webDocs: { | ||
| category: 'Trigger', | ||
| title: 'add specified pipeline/s to the existing or new trigger', |
| const getRoot = require('../../root/get.cmd'); | ||
|
|
||
| const command = new Command({ | ||
| command: 'trigger-types [type] [kind]', |
There was a problem hiding this comment.
lets model the filters: type and kind as options and not arguments
- should be able to get a trigger-type by id/name
codefresh get trigger-types
codefresh get trigger-type ID/NAME
codefresh get trigger-types --type registry
codefresh get trigger-types --kind cfcr
There was a problem hiding this comment.
kind does not make sense without type. And not all types have kinds. E.g. registry type can have dockerhub, ecr, cfcrand other kinds that represent different Docker registries, whilecrontype does not have anykind`.
There was a problem hiding this comment.
that's ok.
still they should be options to filter when you get.
you can define that an option is required with another option if needed
There was a problem hiding this comment.
Will add task to JIRA backlog
| builder: (yargs) => { | ||
| return yargs | ||
| .positional('type', { | ||
| describe: '[optional] trigger type (e.g. registry, cron)', |
There was a problem hiding this comment.
please add 'choices' to the object and put all possible values.
please remove [optional] sign
.positional('typpe', {
describe: 'trigger type',
choices: ['registry', 'cron']
}
There was a problem hiding this comment.
hard. it should be dynamic. It will be possible to add more types and optionally kinds of trigger-types, so it's hard to model them as static "options" list. Maybe use string?
| describe: '[optional] trigger type (e.g. registry, cron)', | ||
| }) | ||
| .positional('kind', { | ||
| describe: '[optional] trigger kind (e.g. dockerhub, cfcr, gcr, acr)', |
There was a problem hiding this comment.
same here with choices
| }, | ||
| webDocs: { | ||
| category: 'Triggers', | ||
| title: 'Get detailed information about trigger event', |
| const getRoot = require('../../root/get.cmd'); | ||
|
|
||
| const command = new Command({ | ||
| command: 'trigger-event <event-uri>', |
There was a problem hiding this comment.
should get all triggers-events or a specific trigger-event by ID/NAME.
event-uri should be a possible filter
There was a problem hiding this comment.
event-URI is the ID for trigger-event
There was a problem hiding this comment.
event-uri is the outcome of creating a trigger-event as i understood.
it should be possible for a user to provide the unique name of the created trigger-event.
if you don't want to also have an id it is fine but the get/delete operations should work with the passed unique name the user will pass.
you can keep the easy way you provided, but it should be possible to get by a specific NAME too.
extended version info (server and trigger manager)
|
Merging now. Need to write a post using current API. |
|
comments |
