Skip to content

external volumes update -> remove SanLun endpoints#740

Merged
blewisCycle merged 10 commits intomainfrom
blewis/extenal-volumes-attachment
Oct 29, 2025
Merged

external volumes update -> remove SanLun endpoints#740
blewisCycle merged 10 commits intomainfrom
blewis/extenal-volumes-attachment

Conversation

@blewisCycle
Copy link
Collaborator

@blewisCycle blewisCycle commented Oct 27, 2025

Update to discriminated union

mattoni
mattoni previously approved these changes Oct 27, 2025
Copy link
Member

@mattoni mattoni left a comment

Choose a reason for hiding this comment

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

Two small cleanup items but LGTM

@@ -0,0 +1,25 @@
# ExternalVolumeAttachmentInstance.yml
Copy link
Member

Choose a reason for hiding this comment

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

I dont think we need this

@@ -0,0 +1,14 @@
# ExternalVolumeAttachment.yml
Copy link
Member

Choose a reason for hiding this comment

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

I dont think we need this

Comment on lines +17 to +34
oneOf:
- type: object
required: [type, details]
properties:
type:
type: string
enum: [block]
details:
$ref: ./ExternalVolumeAttachmentBlock.yml

- type: object
required: [type, details]
properties:
type:
type: string
enum: [filesystem]
details:
$ref: ./ExternalVolumeAttachmentFilesystem.yml
Copy link
Member

Choose a reason for hiding this comment

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

This still should be a discriminated union. You're basing the details on the type.

@blewisCycle blewisCycle changed the title external volumes attachment external volumes update -> remove SanLun endpoints Oct 29, 2025
@blewisCycle blewisCycle merged commit 4fce84f into main Oct 29, 2025
1 check passed
@blewisCycle blewisCycle deleted the blewis/extenal-volumes-attachment branch October 29, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants