-
Notifications
You must be signed in to change notification settings - Fork 97
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
[UDT PR 4 of N] Add basic commands for UDT support #8035
Conversation
/cc @Reshrahim for feedback on command and help text. |
5888cd8
to
7bb1ca2
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
7bb1ca2
to
0daafcb
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
This PR will need a rebase. I'll wait until after getting feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested some changes.
Short: "List resource resource types", | ||
Long: `List resource resource types | ||
|
||
Resource types are the entities that can be created and managed by Radius such as 'Applications.Core/containers'. Resource types can be configured using resource providers.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource types are the entities that can be created and managed by Radius such as 'Applications.Core/containers'. Resource types can be configured using resource providers.`, | |
Resource types are schema definitions for resources that can be created and managed by Radius such as 'Applications.Core/containers'. Resource types can be configured using resource providers.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They aren't really just the schema defintions. Schema is definitely an important part. Let me try rephrasing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example it's not actually the resource types that define a schema. Resource types define api versions, and api versions define schemas.
0daafcb
to
9bcf48c
Compare
This change adds the most basic commands for interacting with resource providers and resource types. Also added some flexibility to existing commands for resources so they can work with UDTs. - `rad resourceprovider` - `list` - `show` - `delete` - `rad resourcetype` - `list` - `show` Also added `rad resource create`. We never implemented anything like this, it was just missing. Also enhanced the `rad resource` family of commands to support any fully-qualified resource type. These commands now how to validate and work with the set of built-in types in Radius today using short names like `containers`. This set of commands are the easy decisions, and already work well using the existing functionality in the implementation. We'll add some of the more complex commands after a design dicussion. Signed-off-by: Ryan Nowak <nowakra@gmail.com>
9bcf48c
to
b0a69ea
Compare
@Reshrahim @ytimocin @nithyatsu - updated |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, clean, easy to follow code. Nothing really to add aside from a semantic nit or two, LGTM
Signed-off-by: Ryan Nowak <nowakra@gmail.com>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Signed-off-by: Ryan Nowak <nowakra@gmail.com>
NewListProviderSummariesPager(planeName string, options *ucpv20231001.ResourceProvidersClientListProviderSummariesOptions) *runtime.Pager[ucpv20231001.ResourceProvidersClientListProviderSummariesResponse] | ||
} | ||
|
||
// resourceProviderClient is an interface for mocking the generated SDK client for resource types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// resourceProviderClient is an interface for mocking the generated SDK client for resource types. | |
// resourceTypeClient is an interface for mocking the generated SDK client for resource types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this. I'm just about to hit send on another PR that will touch this file, so I'll fix it then if that's OK.
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Description
This change adds the most basic commands for interacting with resource providers and resource types. Also added some flexibility to existing commands for resources so they can work with UDTs.
rad resourceprovider
list
show
delete
rad resourcetype
list
show
Also added
rad resource create
. We never implemented anything like this, it was just missing.Also enhanced the
rad resource
family of commands to support any fully-qualified resource type. These commands now how to validate and work with the set of built-in types in Radius today using short names likecontainers
.This set of commands are the easy decisions, and already work well using the existing functionality in the implementation. We'll add some of the more complex commands after a design discussion.
Type of change
Part of: #6688
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: