Switch to UUID for event IDs #926
Conversation
tekton-robot
added
the
release-note-action-required
tekton-robot
added
the
size/L
| // UID generates a random string like the Kubernetes apiserver generateName metafield postfix. | ||
| var UID = func() string { return rand.String(5) } | ||
| // UUID generates a Universally Unique IDentifier following RFC 4122. | ||
| var UUID = func() string { return uuid.New().String() } |
There was a problem hiding this comment.
I have currently changed the naming everywhere from uid to uuid - I am not certain if this is really required.
uuid is more clear as to what it is.
uid is a less breaking change (and is the name k8s uses AFAIK).
There was a problem hiding this comment.
cc @wlynch in case you have suggestions.
There was a problem hiding this comment.
I agree - I'm leaning towards leaving things as uid based on your reasoning + so we don't have to bother with deprecation.
There was a problem hiding this comment.
This is done now - the change is not only very internal, renaming only the internal functions properly.
| // UID generates a random string like the Kubernetes apiserver generateName metafield postfix. | ||
| var UID = func() string { return rand.String(5) } | ||
| // UUID generates a Universally Unique IDentifier following RFC 4122. | ||
| var UUID = func() string { return uuid.New().String() } |
There was a problem hiding this comment.
I agree - I'm leaning towards leaving things as uid based on your reasoning + so we don't have to bother with deprecation.
tekton-robot
added
size/M
MarcelMue
force-pushed
the
switch-uuid
branch
from
d89a179 to
afe457f
Compare
tekton-robot
added
release-note
|
I am not aware of any docs which should be changed. None that I could find mention the ID structure. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wlynch 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 |
tekton-robot
added
the
approved
resulting from our use of $(uid) with tektoncd/triggers#926. Long pod names work, but usability suffers. In particular with tasks in a pipeline we no longer see the task name in kubectl watch. For Dashboard it's also impractical because the pod name is the headline of the taskrun page. A solution could be based on generateName, like Triggers' PR 939.
Changes
Switches the 5 character random string UIDs to RFC 4122 UUIDs.
Closes: #901
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Release Notes