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

Support Triggers 0.2 features in the dashboard #944

Closed
a-roberts opened this issue Jan 23, 2020 · 10 comments · Fixed by #994
Closed

Support Triggers 0.2 features in the dashboard #944

a-roberts opened this issue Jan 23, 2020 · 10 comments · Fixed by #994
Assignees
Milestone

Comments

@a-roberts
Copy link
Member

a-roberts commented Jan 23, 2020

With the assessment done, under this task go ahead and do it, so specifically we want to:

  1. Install Triggers 0.2
  2. Read through Assess the move to Triggers 0.2 (Dashboard) #918 and Front End Development for Triggers UI #937
  3. Make the changes and test/contribute

The assessment task (#918) should include all of the changes we'll have to make. If it does turn into something many folks can work on at once it'll be great to add more issues so we can get this done asap 😸

Obvious things: multiple bindings and interceptors

@a-roberts a-roberts added this to the v0.5.0 milestone Jan 27, 2020
@ncskier
Copy link
Member

ncskier commented Jan 27, 2020

/assign

@kimholmes
Copy link

@AlanGreene and @ncskier Per Brandon's note in this older issue:
#918 (comment)
"Multiple interceptors
EventListener triggers can now have multiple interceptors instead of just one. So, we must display this list of interceptors."

Does this mean the UI could turn up multiple headers per interceptor? We might need to tweak the UI if this becomes unruly due to scrolling etc.
See Carrie's current mock-ups:
#937

@ncskier
Copy link
Member

ncskier commented Jan 28, 2020

Does this mean the UI could turn up multiple headers per interceptor?

@kimholmes not multiple headers per interceptor, but yes multiple headers per EventListener. Each interceptor has its own headers, and each EventListener can have multiple interceptors. So we could have an EventListener that looks like this:

EventListener
- Interceptor-1
  - List of headers for Interceptor-1
- Interceptor-2
  - List of headers for Interceptor-2
- Interceptor-3
  - List of headers for Interceptor-3
...

Does that answer your question? 🙂

@kimholmes
Copy link

@ncskier Yes, for some reason, I was under the impression that the headers were a child of the Interceptor. I hope the UI isn't confusing.

@ncskier
Copy link
Member

ncskier commented Jan 28, 2020

Yes, the headers are a child of each interceptor:

EventListener
- Interceptor-1
  - List of headers for Interceptor-1
- Interceptor-2
  - List of headers for Interceptor-2
- Interceptor-3
  - List of headers for Interceptor-3

@ncskier
Copy link
Member

ncskier commented Jan 29, 2020

@a-roberts @AlanGreene, how do we want to address backwards-compatibility? Since Triggers only has a few early alpha releases, we're assuming that there will be some breaking changes. So, I think that it makes sense to not worry about supporting v0.1.0 Triggers resources in the Dashboard.

I just want to verify that it is OK if my implementation of this issue will mean the Dashboard "breaks" when displaying v0.1.0 Triggers resources.

@AlanGreene
Copy link
Member

AlanGreene commented Jan 29, 2020

What are the plans for dashboard 0.5 (inc. webhooks extension)? I see a draft release referencing triggers 0.1, do we need to get that out before bumping the triggers version or are we ok to go ahead with the update now?

@a-roberts
Copy link
Member Author

@ncskier @AlanGreene good points, I wanted to get in @Megan-Wright's PR for the proper displaying of proxy request errors (mainly from a debug/serviceability perspective), so I suggest we do a 0.5.0 release that's for Triggers 0.1 and Pipeline 0.10.1 one shortly (see: today).

Then the 0.5.1 release is for Triggers 0.2 once that's ready, this gives folks a chance to use either Triggers version with a supported release.

@a-roberts a-roberts modified the milestones: v0.5.0, v0.5.1 Jan 30, 2020
@ncskier
Copy link
Member

ncskier commented Jan 30, 2020

Ok, thanks for the clarification Adam! 👍

@ncskier
Copy link
Member

ncskier commented Jan 31, 2020

Status update:

I can now display multiple bindings, and interceptors. Plus there are no more params. Here's a screenshot (note that I copied and pasted the interceptors, so they have the same name). I used an accordion to display the multiple interceptors for now, because it was easy and saves space. Feedback is always welcome 🙂 However, keep in mind the styles will probably change with issue #937.

Screen Shot 2020-01-31 at 4 36 24 PM

Still to do:

  • Support displaying the 3 additional built-in Triggers Interceptors (GitHub, GitLab, and CEL)
  • Update tests
  • Fix bugs & edge cases that I've noticed; for example, displaying Triggers without a name & displaying ServiceAccount

ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 4, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 4, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 5, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 5, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 5, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 5, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 5, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 5, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 7, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
ncskier pushed a commit to ncskier/dashboard that referenced this issue Feb 7, 2020
Fixes tektoncd#944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
tekton-robot pushed a commit that referenced this issue Feb 7, 2020
Fixes #944

Updates the EventListener details page with the following:
- Multiple Interceptors
- Multiple TriggerBindings
- No more params
- Display all 4 built-in types of Interceptors (Webhook, GitHub, GitLab,
and CEL)
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 a pull request may close this issue.

4 participants