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.

Sign up for GitHub

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

Jump to bottom

Add code completion for when clause #223

Merged
merged 8 commits into from
Sep 30, 2020
Merged

Add code completion for when clause #223

merged 8 commits into from
Sep 30, 2020

Conversation

lstocchi
Copy link
Contributor

@lstocchi lstocchi commented Sep 18, 2020 •
edited
Loading

it resolves #209

@jeffmaury this patch was born to add code completion for when clause but it is starting going too far so i prefer to stop here and complete the initial work in further PRs.

This patch contains two things essentially.

  1. the actual code completion for when clause -> this shows the in, notin operator options
  2. Additional features useful for the when clause:
    • code completion for tasks results (often used as input/value in a when clause)
    • implementation of the virtual file system to avoid downloading a task multiple times when unnecessary

Once this will be merged i will extend the usage of the vfs to everything else and add tests for the vfs.

An example of how it works below:

whenoperator

@lstocchi
fix issue with generic code completion that is not displayed in list
a6fdfe7 
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
@lstocchi
add initial code completion for when clause (redhat-developer#209)
81ff9dd 
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
@lstocchi
restore old test content
8ecb2df 
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
@lstocchi
add tests
17313de 
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
@lstocchi
add virtual file system
3e3d9ab 
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
@lstocchi
clean
751098a 
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
@lstocchi lstocchi marked this pull request as ready for review September 23, 2020 08:08
@lstocchi lstocchi requested a review from jeffmaury September 23, 2020 08:08
jeffmaury
jeffmaury requested changes Sep 24, 2020
View reviewed changes
Copy link
Member

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

According to the source code, the values for Operator are the following:


	!
	=
	==
	in
	!=
	notin
	exists
	gt
	lt

But the doc mentions only in and notin.
So asking @jerop for confirmation

@jerop
Copy link

jerop commented Sep 24, 2020

According to the source code, the values for Operator are the following:


	!
	=
	==
	in
	!=
	notin
	exists
	gt
	lt

But the doc mentions only in and notin.
So asking @jerop for confirmation

@jeffmaury even though Operator has all those options, we only support in and notin in When Expressions given the use cases we looked at for guarding Task execution

If another Operator is provided, the Pipeline validation would fail and throw an error

We could consider supporting other Operators as needed, based on the use cases, in the future

@jerop
Copy link

jerop commented Sep 24, 2020

@lstocchi please note that input, operator and values are supposed to be lowercase in the yaml -- it seems in the implementation and example of the completion that they're pascalcase 🤔

@jeffmaury
Copy link
Member

Yes that a side effect because our JSON schema generator is parsing the Go code and I noticed that the WhenExpression type does not have the json annotation like other types in the pipeline code. @jerop let me know if I should open an issue

@lstocchi
Copy link
Contributor Author

Yes that a side effect because our JSON schema generator is parsing the Go code and I noticed that the WhenExpression type does not have the json annotation like other types in the pipeline code. @jerop let me know if I should open an issue

I didn't pay much attention on this aspect before but now that @jerop pointed that out i guess we need to fix it. If someone copy/paste an example from the tekton repo (which contains the lowercase names), she/he will get a warning in our plugin. Not good.

@jerop jerop mentioned this pull request Sep 28, 2020
Merged
4 tasks
@jerop
Copy link

jerop commented Sep 28, 2020

Yes that a side effect because our JSON schema generator is parsing the Go code and I noticed that the WhenExpression type does not have the json annotation like other types in the pipeline code. @jerop let me know if I should open an issue

@jeffmaury added the annotations in tektoncd/pipeline#3291

excited to see code completion for when expressions supported in the plugin!

@lstocchi
fix when operator completion uppercase
442251b 
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
@lstocchi
fixed test
31270f8 
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
@lstocchi lstocchi requested a review from jeffmaury September 29, 2020 16:27
@lstocchi lstocchi merged commit bcd19ff into redhat-developer:master Sep 30, 2020
@jerop jerop mentioned this pull request Oct 14, 2020
Merged
4 tasks
jerop added a commit to jerop/pipeline that referenced this pull request Oct 19, 2020
@jerop
Fix json annotations in WhenExpression
84734ee 
When a pipeline with when expressions is created in v0.16.* then it is
run in a pipelinerun in v0.17.0 or v0.17.1, a pipeline validation error
about missing fields would be thrown -- as reported in tektoncd#3382

That happened because json annotations were added to when expressions
type in v0.17.0 so that the fields would have lowercase, such as in the
code completion in tekton intellij plugin as described in
redhat-developer/intellij-tekton#223

Without the json annotations, the fields were stored with first letters
capitalized, that is Input, Operator and Values. With the json
annotations, the fields were expected to be lowercase, that is input,
operator and values, causing the missing fields error in pipeline
validation

As such, users would have to reapply pipelines definitions created in
previous versions to make them work in v0.17.0 or v0.17.1 -- to remove
this requirement, we need to support both the uppercase and lowercase
first letters for the annotations

Fixes tektoncd#3382
jerop added a commit to jerop/pipeline that referenced this pull request Oct 19, 2020
@jerop
Fix json annotations in WhenExpression
4033ab8 
When a pipeline with when expressions is created in v0.16.* then it is
run in a pipelinerun in v0.17.0 or v0.17.1, a pipeline validation error
about missing fields would be thrown -- as reported in tektoncd#3382

That happened because json annotations were added to when expressions
type in v0.17.0 so that the fields would have lowercase, such as in the
code completion in tekton intellij plugin as described in
redhat-developer/intellij-tekton#223

Without the json annotations, the fields were stored with first letters
capitalized, that is Input, Operator and Values. With the json
annotations, the fields were expected to be lowercase, that is input,
operator and values, causing the missing fields error in pipeline
validation

As such, users would have to reapply pipelines definitions created in
previous versions to make them work in v0.17.0 or v0.17.1 -- to remove
this requirement, we need to support both the uppercase and lowercase
first letters for the annotations

Fixes tektoncd#3382
jerop added a commit to jerop/pipeline that referenced this pull request Oct 19, 2020
@jerop
Fix json annotations in WhenExpression
570cc86 
When a pipeline with when expressions is created in v0.16.* then it is
run in a pipelinerun in v0.17.0 or v0.17.1, a pipeline validation error
about missing fields would be thrown -- as reported in tektoncd#3382

That happened because json annotations were added to when expressions
type in v0.17.0 so that the fields would have lowercase, such as in the
code completion in tekton intellij plugin as described in
redhat-developer/intellij-tekton#223

Without the json annotations, the fields were stored with first letters
capitalized, that is Input, Operator and Values. With the json
annotations, the fields were expected to be lowercase, that is input,
operator and values, causing the missing fields error in pipeline
validation

As such, users would have to reapply pipelines definitions created in
previous versions to make them work in v0.17.0 or v0.17.1 -- to remove
this requirement, we need to support both the uppercase and lowercase
first letters for the annotations

Fixes tektoncd#3382
jerop added a commit to jerop/pipeline that referenced this pull request Oct 19, 2020
@jerop
Fix json annotations in WhenExpression
4f35ed1 
When a pipeline with when expressions is created in v0.16.* then it is
run in a pipelinerun in v0.17.0 or v0.17.1, a pipeline validation error
about missing fields would be thrown -- as reported in tektoncd#3382

That happened because json annotations were added to when expressions
type in v0.17.0 so that the fields would have lowercase, such as in the
code completion in tekton intellij plugin as described in
redhat-developer/intellij-tekton#223

Without the json annotations, the fields were stored with first letters
capitalized, that is Input, Operator and Values. With the json
annotations, the fields were expected to be lowercase, that is input,
operator and values, causing the missing fields error in pipeline
validation

As such, users would have to reapply pipelines definitions created in
previous versions to make them work in v0.17.0 or v0.17.1 -- to remove
this requirement, we need to support both the uppercase and lowercase
first letters for the annotations

Fixes tektoncd#3382
tekton-robot pushed a commit to tektoncd/pipeline that referenced this pull request Oct 21, 2020
@jerop @tekton-robot
Fix json annotations in WhenExpression
4edcbc1 
When a pipeline with when expressions is created in v0.16.* then it is
run in a pipelinerun in v0.17.0 or v0.17.1, a pipeline validation error
about missing fields would be thrown -- as reported in #3382

That happened because json annotations were added to when expressions
type in v0.17.0 so that the fields would have lowercase, such as in the
code completion in tekton intellij plugin as described in
redhat-developer/intellij-tekton#223

Without the json annotations, the fields were stored with first letters
capitalized, that is Input, Operator and Values. With the json
annotations, the fields were expected to be lowercase, that is input,
operator and values, causing the missing fields error in pipeline
validation

As such, users would have to reapply pipelines definitions created in
previous versions to make them work in v0.17.0 or v0.17.1 -- to remove
this requirement, we need to support both the uppercase and lowercase
first letters for the annotations

Fixes #3382
vdemeester pushed a commit to vdemeester/tektoncd-pipeline that referenced this pull request Oct 21, 2020
@jerop @vdemeester
Fix json annotations in WhenExpression
7654ddb 
When a pipeline with when expressions is created in v0.16.* then it is
run in a pipelinerun in v0.17.0 or v0.17.1, a pipeline validation error
about missing fields would be thrown -- as reported in tektoncd#3382

That happened because json annotations were added to when expressions
type in v0.17.0 so that the fields would have lowercase, such as in the
code completion in tekton intellij plugin as described in
redhat-developer/intellij-tekton#223

Without the json annotations, the fields were stored with first letters
capitalized, that is Input, Operator and Values. With the json
annotations, the fields were expected to be lowercase, that is input,
operator and values, causing the missing fields error in pipeline
validation

As such, users would have to reapply pipelines definitions created in
previous versions to make them work in v0.17.0 or v0.17.1 -- to remove
this requirement, we need to support both the uppercase and lowercase
first letters for the annotations

Fixes tektoncd#3382

(cherry picked from commit 4edcbc1)
Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
@vdemeester vdemeester mentioned this pull request Oct 21, 2020
Merged
4 tasks
tekton-robot pushed a commit to tektoncd/pipeline that referenced this pull request Oct 21, 2020
@jerop @tekton-robot
Fix json annotations in WhenExpression
5313762 
When a pipeline with when expressions is created in v0.16.* then it is
run in a pipelinerun in v0.17.0 or v0.17.1, a pipeline validation error
about missing fields would be thrown -- as reported in #3382

That happened because json annotations were added to when expressions
type in v0.17.0 so that the fields would have lowercase, such as in the
code completion in tekton intellij plugin as described in
redhat-developer/intellij-tekton#223

Without the json annotations, the fields were stored with first letters
capitalized, that is Input, Operator and Values. With the json
annotations, the fields were expected to be lowercase, that is input,
operator and values, causing the missing fields error in pipeline
validation

As such, users would have to reapply pipelines definitions created in
previous versions to make them work in v0.17.0 or v0.17.1 -- to remove
this requirement, we need to support both the uppercase and lowercase
first letters for the annotations

Fixes #3382

(cherry picked from commit 4edcbc1)
Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffmaury jeffmaury jeffmaury approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add code completion for when clause
3 participants
@lstocchi @jerop @jeffmaury