-
Notifications
You must be signed in to change notification settings - Fork 36
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
Schema vs Doc inconsistency regarding key
vs identifier
vs id
attributes?
#76
Comments
Wonder if https://json-schema.org/draft/2019-09/json-schema-validation#rfc.section.9.3 could be leveraged |
FWIW just tried a pipeline with Additionally, you're (seemingly) allowed to pass multiple and |
Last bit: all of the above have the same "cannot look like UUID" restriction. |
Thanks for the investigation! I'm surprised that defining a Clearly some documentation and schema update needed to clarify all that anyway |
Ok, PR up. I also made one for the docs: buildkite/docs#3011 |
The inconsistency
#/definitions/commonOptions/identifier
(whose description is"A string identifier"
), that is then used as reference for both attributesid
andidentifier
in steps#/definitions/commonOptions/key
(whose description is"A unique identifier for a step, must not resemble a UUID"
), that is used as reference for thekey
attribute in stepskey
, and thatidentifier
is a valid alias forkey
, while not mentioningid
.My understanding of the current state
I believe the doc is correct, as it matches the behavior we see in practice.
My guess is that maybe (?)
key
was namedid
/identifier
a long time ago, but Buildkite changed it tokey
since, and kept theidentifier
as an alias for backwards compatibility… while makingid
private because it would probably now be a generated UUID (the one corresponding to theBUILDKITE_STEP_ID
env var value?) instead of a user-provided one. So they updated the docs accordingly… but forgot to fully update the schema in line, only adding a definition for thekey
attribute but not updating the schema definition to removeid
nor to makeidentifier
backwards-compat alias now point to#/definitions/commonOptions/key
instead of#/definitions/commonOptions/identifier
?The text was updated successfully, but these errors were encountered: