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

TEP-0075: Add design proposal to handle edge cases #711

Merged
merged 1 commit into from
Aug 29, 2022
Merged

TEP-0075: Add design proposal to handle edge cases #711

merged 1 commit into from
Aug 29, 2022

Conversation

chuangw6
Copy link
Member

  • Collision when other parameter names contain dots .
  • Problem when the object parameter name itself or its key name
    contains dots .

@tekton-robot tekton-robot requested review from dibbles and lbernick May 26, 2022 00:23
@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 26, 2022
@chuangw6
Copy link
Member Author

/assign @wlynch
/assign @jerop

@chuangw6 chuangw6 force-pushed the update-tep75 branch 2 times, most recently from b1b3e7a to cb8c598 Compare May 26, 2022 00:35
@chuangw6
Copy link
Member Author

/assign @lbernick
/assign @ywluogg

@tekton-robot
Copy link
Contributor

@chuangw6: GitHub didn't allow me to assign the following users: ywluogg.

Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @lbernick
/assign @ywluogg

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@lbernick
Copy link
Member

lbernick commented May 27, 2022 •
edited
Loading

@chuangw6 thanks for these updates! would you mind discussing how this impacts results as well as params? I remember you mentioned we don't support dots for results (but not completely sure if I'm right about this).

@mattmoor this is probably also relevant to you since you wrote the TEP introducing dots in params

@ywluogg
Copy link
Contributor

ywluogg commented Jun 2, 2022

/assign ywluogg

@ywluogg
Copy link
Contributor

ywluogg commented Jun 2, 2022

Link this to #4723

@jerop
Copy link
Member

jerop commented Jun 6, 2022

/kind tep

@tekton-robot tekton-robot added the kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). label Jun 6, 2022
chuangw6 added a commit to chuangw6/pipeline that referenced this pull request Jul 6, 2022
@chuangw6
TEP-0075: Validate object name and key name have no dots
1b6ecd5 
Prior to this commit, dots are allowed to be used in param names to
support domain-scoped names. However, object params have already
supported domain-scoped names since it has a list of keys. In addition,
using dots in object param names and key names have conflicts.
See more details in tektoncd/community#711.

In this change, we validate against those object names and key names
that contain dots.
@chuangw6 chuangw6 mentioned this pull request Jul 6, 2022
Merged
5 tasks
chuangw6 added a commit to chuangw6/pipeline that referenced this pull request Jul 8, 2022
@chuangw6
TEP-0075: Validate object name and key name have no dots
9a9c70a 
Prior to this commit, dots are allowed to be used in param names to
support domain-scoped names. However, object params have already
supported domain-scoped names since it has a list of keys. In addition,
using dots in object param names and key names have conflicts.
See more details in tektoncd/community#711.

In this change, we validate against those object names and key names
that contain dots.
chuangw6 added a commit to chuangw6/pipeline that referenced this pull request Jul 11, 2022
@chuangw6
TEP-0075: Validate object name and key name have no dots
c7464bf 
Prior to this commit, dots are allowed to be used in param names to
support domain-scoped names. However, object params have already
supported domain-scoped names since it has a list of keys. In addition,
using dots in object param names and key names have conflicts.
See more details in tektoncd/community#711.

In this change, we validate against those object names and key names
that contain dots.
chuangw6 added a commit to chuangw6/pipeline that referenced this pull request Jul 11, 2022
@chuangw6
TEP-0075: Validate object name and key name have no dots
ba212d3 
Prior to this commit, dots are allowed to be used in param names to
support domain-scoped names. However, object params have already
supported domain-scoped names since it has a list of keys. In addition,
using dots in object param names and key names have conflicts.
See more details in tektoncd/community#711.

In this change, we validate against those object names and key names
that contain dots.
chuangw6 added a commit to chuangw6/pipeline that referenced this pull request Jul 11, 2022
@chuangw6
TEP-0075: Validate object name and key name have no dots
8122b24 
Prior to this commit, dots are allowed to be used in param names to
support domain-scoped names. However, object params have already
supported domain-scoped names since it has a list of keys. In addition,
using dots in object param names and key names have conflicts.
See more details in tektoncd/community#711.

In this change, we validate against those object names and key names
that contain dots.
tekton-robot pushed a commit to tektoncd/pipeline that referenced this pull request Jul 11, 2022
@chuangw6 @tekton-robot
TEP-0075: Validate object name and key name have no dots
9dacb49 
Prior to this commit, dots are allowed to be used in param names to
support domain-scoped names. However, object params have already
supported domain-scoped names since it has a list of keys. In addition,
using dots in object param names and key names have conflicts.
See more details in tektoncd/community#711.

In this change, we validate against those object names and key names
that contain dots.
@dibyom
Copy link
Member

dibyom commented Aug 11, 2022

/cc @chuangw6 @ywluogg @lbernick @jerop what are we planning to do with this PR?

@ywluogg
Copy link
Contributor

ywluogg commented Aug 11, 2022

/cc @chuangw6 @ywluogg @lbernick @jerop what are we planning to do with this PR?

This is for adding some clarifications when param / object names are having . dots involved. I still haven't finished my review of this yet...

@chuangw6
TEP-0075: Add design proposal to handle edge cases
1cf2abe 
- Collision when other parameter names contain dots .
- Problem when the object parameter name itself or its key name
contains dots .
@dibyom
Copy link
Member

dibyom commented Aug 15, 2022

API WG: Waiting on reviews

@chuangw6
Copy link
Member Author

Hi @ywluogg ,
Please take a look. Let me know if you have any comments.
Thanks.

@chuangw6
Copy link
Member Author

Hi @jerop ,
Please take a look. Thanks.

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2022
@jerop
Copy link
Member

jerop commented Aug 22, 2022

API WG 08/22

need a non-googler approver, please take a look @wlynch

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick, vdemeester, ywluogg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@pritidesai
Copy link
Member

API WG - ready to merge

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 29, 2022
@tekton-robot tekton-robot merged commit 59e0b72 into tektoncd:main Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ywluogg ywluogg ywluogg approved these changes

@lbernick lbernick lbernick approved these changes

@vdemeester vdemeester vdemeester approved these changes

@dibbles dibbles Awaiting requested review from dibbles

Assignees

@pritidesai pritidesai

@wlynch wlynch

@jerop jerop

@lbernick lbernick

@ywluogg ywluogg

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
TEP Steward Assignments
Status: UnAssigned
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@chuangw6 @tekton-robot @lbernick @ywluogg @jerop @dibyom @pritidesai @vdemeester @wlynch