[nexus] Accept DiskSource::{Image, Snapshot}s without read_only#9766
Merged
[nexus] Accept DiskSource::{Image, Snapshot}s without read_only#9766
DiskSource::{Image, Snapshot}s without read_only#9766Conversation
@ahl rightly [points out][1] that the common case for the `disk_create` APIs is to create read-write disks, and it seems unfortunate to require all calls to create a read-write disk from an image or snapshot to require a `"read_only": false` field in the JSON body. Thus, this commit adds `#[serde(default)]` so that disk source params _without_ a `"read_only"` are the same as `"read_only": false`. Unfortunately, we didn't catch this until _after_ #9731 merged, so fixing it the proper way requires a whole new API version. Since the conversion between the Rust params types in `nexus-types` is trivial, it might have been possible to bend the rules a bit here and just tweak the previously-generated API version to make the field nullable --- clients operating with the slightly-older version of that version would still always be accepted since they would _always_ send the field. But, bending the rules makes me scared I'm gonna get in trouble, so I did it the proper way, at the expense of a whole lot of code that does basically nothing.
DiskSource::{Inmage, Snapshot}s without read_onlyDiskSource::{Image, Snapshot}s without read_only
david-crespo
approved these changes
Jan 31, 2026
Contributor
david-crespo
left a comment
There was a problem hiding this comment.
Looks fine to me. Can’t believe the conversions. In theory it could be fine to just overwrite the old version since nobody is running the previous version. It does feel completely ridiculous to have to do this.
Contributor
|
Confirmed it has the desired effect on the TS client: --- a/2026013001.0.0-1c5abd/Api.ts
+++ b/2026013100.0.0-c09e48/Api.ts
@@ -1920,7 +1920,7 @@
/** Create a disk from a disk snapshot */
| {
/** If `true`, the disk created from this snapshot will be read-only. */
- readOnly: boolean;
+ readOnly?: boolean;
snapshotId: string;
type: "snapshot";
}
@@ -1928,7 +1928,7 @@
| {
imageId: string;
/** If `true`, the disk created from this image will be read-only. */
- readOnly: boolean;
+ readOnly?: boolean;
type: "image";
}
/** Create a blank disk that will accept bulk writes or pull blocks from an external source. */
@@ -7484,7 +7484,7 @@
* Pulled from info.version in the OpenAPI schema. Sent in the
* `api-version` header on all requests.
*/
- apiVersion = "2026013001.0.0";
+ apiVersion = "2026013100.0.0";
constructor({ host = "", baseParams = {}, token }: ApiConfig = {}) {
this.host = host; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@ahl rightly points out that the common case for the
disk_createAPIs is to create read-write disks, and it seems like kind of a drag to require all calls to create a read-write disk from an image or snapshot to include a"read_only": falsefield in the JSON body. Thus, this commit adds#[serde(default)]so that disk source params without a"read_only"are the same as"read_only": false.Unfortunately, we didn't catch this until after #9731 merged, so fixing it the proper way requires a whole new API version. Since the conversion between the Rust params types in
nexus-typesis trivial, it might have been possible to bend the rules a bit here and just tweak the previously-generated API version to make the field nullable --- clients operating with the slightly-older version of that version would still always be accepted since they would always send the field. But, bending the rules makes me scared I'm gonna get in trouble, so I did it the proper way, at the expense of a whole lot of code that does basically nothing.