Create ResourceLabels and use for ManagedResource, NetworkConfiguration.#1360
Create ResourceLabels and use for ManagedResource, NetworkConfiguration.#1360jglogan merged 2 commits intoapple:mainfrom
Conversation
|
|
||
| import Collections | ||
|
|
||
| public protocol AppError: Error { |
There was a problem hiding this comment.
Is this going to be the used for the highest level and for generic type of errors (that all resources will follow)? I kind of worked on creating more lower level, managed resource specific handling approach (that each resource will follow but will vary per resource).
There was a problem hiding this comment.
I don't know the answer to that for sure yet. I feel like we'll want to try as much as we can in the coming month and evolve to something sensible by the next release.
What I was going for with this error is:
- Supports a caused-by chain.
- Supports structured log output
- Delegates presentation to an error receiver that is responsible for making sense of chained errors, compound errors, and error metadata and mapping it to one or more log messages.
- For easier compatibility, doesn't rely on
enumfor error typing.
There was a problem hiding this comment.
Sounds good to me. I am going to take a step back for now and let you lead this. Excited to see what it becomes! Happy to jump back in when it’s ready to be used for managed resources.
- Create a ResourceLabels type and extract the label validation from NetworkConfiguration into the new type. - Create a base AppError type that is compatible with structured logging and delegates message presentation to the error receiver. - Define LabelError over AppError for label validation. - Slightly reworks NetworkConfiguration entity migration code in NetworksService.
ajemory
left a comment
ajemory
left a comment
There was a problem hiding this comment.
Two stray comments, otherwise looks good
| @@ -53,6 +53,6 @@ extension ManagedResource { | |||
| } | |||
|
|
|||
| // FIXME: This moves to ManagedResource and/or a ResourceLabels typealias eventually. | |||
There was a problem hiding this comment.
| // FIXME: This moves to ManagedResource and/or a ResourceLabels typealias eventually. |
| @@ -33,7 +33,7 @@ public protocol ManagedResource: Identifiable, Sendable, Codable { | |||
| /// Key-value properties for the resource. The user and system may both | |||
| /// make use of labels to read and write annotations or other metadata. | |||
| /// A good practice is to use | |||
4 participants
Type of Change
Motivation and Context
See related issue.
Testing