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 conventions about primitive types. #17377

Merged
merged 2 commits into from
Nov 19, 2015
Merged

Add conventions about primitive types. #17377

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0141c53
Add conventions about primitive types.
bgrant0607 Nov 17, 2015
d625217
Address feedback
bgrant0607 Nov 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/devel/api-conventions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ using resources with kubectl can be found in [Working with resources](../user-gu
- [Typical status properties](#typical-status-properties)
- [References to related objects](#references-to-related-objects)
- [Lists of named subobjects preferred over maps](#lists-of-named-subobjects-preferred-over-maps)
- [Primitive types](#primitive-types)
- [Constants](#constants)
- [Lists and Simple kinds](#lists-and-simple-kinds)
- [Differing Representations](#differing-representations)
Expand Down Expand Up @@ -247,6 +248,14 @@ ports:

This rule maintains the invariant that all JSON/YAML keys are fields in API objects. The only exceptions are pure maps in the API (currently, labels, selectors, annotations, data), as opposed to sets of subobjects.

#### Primitive types

* Avoid floating-point values as much as possible, and never use them in spec. Floating-point values cannot be reliably round-tripped (encoded and re-decoded) without changing, and have varying precision and representations across languages and architectures.
* All numbers (e.g., uint32, int64) are converted to float64 by Javascript and some other languages, so any field which is expected to exceed that either in magnitude or in precision (specifically integer values > 53 bits) should be serialized and accepted as strings.
* Do not use unsigned integers, due to inconsistent support across languages and libraries. Just validate that the integer is non-negative if that's the case.
Copy link
Member

Choose a reason for hiding this comment

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

@smarterclayton I sort of like this... int64 validated to be 0 <= x <= 2^32 instead of uint32? what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I could be ok with this - @thockin if you agree I'll use int64 internally
for UID/GID and then we'll validate to the range

On Wed, Nov 18, 2015 at 1:05 PM, Jordan Liggitt notifications@github.com
wrote:

In docs/devel/api-conventions.md
#17377 (comment)
:

@@ -247,6 +248,14 @@ ports:

This rule maintains the invariant that all JSON/YAML keys are fields in API objects. The only exceptions are pure maps in the API (currently, labels, selectors, annotations, data), as opposed to sets of subobjects.

+#### Primitive types
+
+* Avoid floating-point values as much as possible, and never use them in spec. Floating-point values cannot be reliably round-tripped (encoded and re-decoded) without changing, and have varying precision and representations across languages and architectures.
+* All numbers (e.g., uint32, int64) are converted to float64 by Javascript and some other languages, so any field which is expected to exceed that either in magnitude or in precision (specifically integer values > 53 bits) should be serialized and accepted as strings.
+* Do not use unsigned integers, due to inconsistent support across languages and libraries. Just validate that the integer is non-negative if that's the case.

@smarterclayton https://github.com/smarterclayton I sort of like
this... int64 validated to be 0 <= x <= 2^32 instead of uint32? what do
you think?

—
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/17377/files#r45235687.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could make exceptions, if compelling, but I'd rather not. We haven't run tests in Javascript, Python, PHP, Ruby, Java, etc., nor in various parsing/printing libraries, nor alternate encodings (e.g., yaml, ubjson), but it's definitely the case that some don't natively support unsigned ints. For instance, it looks like Java would need to parse unit32 as a double (http://www.json.org/javadoc/org/json/JSONObject.html), in which case it would have to validate sign and size, anyway.

We don't have any examples of bitmasks yet, but I'd be inclined to say they should be strings.

Copy link
Member

Choose a reason for hiding this comment

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

I can live with this, I think

Copy link
Contributor

Choose a reason for hiding this comment

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

We kind of do - the range allocators are base64 encoded bitmask strings.
Not publicly exposed yet.

On Nov 19, 2015, at 12:22 AM, Brian Grant notifications@github.com wrote:

In docs/devel/api-conventions.md
#17377 (comment):

@@ -247,6 +248,14 @@ ports:

This rule maintains the invariant that all JSON/YAML keys are fields in API objects. The only exceptions are pure maps in the API (currently, labels, selectors, annotations, data), as opposed to sets of subobjects.

+#### Primitive types
+
+* Avoid floating-point values as much as possible, and never use them in spec. Floating-point values cannot be reliably round-tripped (encoded and re-decoded) without changing, and have varying precision and representations across languages and architectures.
+* All numbers (e.g., uint32, int64) are converted to float64 by Javascript and some other languages, so any field which is expected to exceed that either in magnitude or in precision (specifically integer values > 53 bits) should be serialized and accepted as strings.
+* Do not use unsigned integers, due to inconsistent support across languages and libraries. Just validate that the integer is non-negative if that's the case.

We could make exceptions, if compelling, but I'd rather not. We haven't run
tests in Javascript, Python, PHP, Ruby, Java, etc., nor in various
parsing/printing libraries, nor alternate encodings (e.g., yaml, ubjson),
but it's definitely the case that some don't natively support unsigned
ints. For instance, it looks like Java would need to parse unit32 as a
double (http://www.json.org/javadoc/org/json/JSONObject.html), in which
case it would have to validate sign and size, anyway.

We don't have any examples of bitmasks yet, but I'd be inclined to say they
should be strings.

—
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/17377/files#r45301195.

* Do not use enums. Use aliases for string instead (e.g., `NodeConditionType`).
Copy link
Member

Choose a reason for hiding this comment

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

Tangent: we don't validate the fields typed as string aliases are one of the defined constants for that type. Should we? Go string aliases can take any value and aren't bound by const blocks.

Copy link
Member

Choose a reason for hiding this comment

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

validation logic should check that values are valid. I know we do for a lot of fields (I wrote it). We might not be 100%

* Look at similar fields in the API (e.g., ports, durations) and follow the conventions of existing fields.

#### Constants

Some fields will have a list of allowed values (enumerations). These values will be strings, and they will be in CamelCase, with an initial uppercase letter. Examples: "ClusterFirst", "Pending", "ClientIP".
Expand Down