feat(md): Types and new data source for environments #8020
Conversation
| pending: string[]; | ||
| approved: string[]; | ||
| previous: string[]; | ||
| vetoed: string[]; |
| versions: IManagedArtifactVersion[]; | ||
| } | ||
|
|
||
| export interface IManagedApplicationEnvironmentsSummary extends IManagedApplicationSummary { |
There was a problem hiding this comment.
I think you could do something like this:
type IManagedApplicationSummary<
T extends keyof IManagedApplicationEntities = 'resources'
> = Pick<IManagedApplicationEntities, T> & {
applicationPaused: boolean;
hasManagedResources: boolean;
}
interface IManagedApplicationEntities {
resources: IManagedResourceSummary[];
environments: IManagedEnviromentSummary[];
artifacts: IManagedArtifactSummary[];
}And then in the reader:
class ManagedReader {
public static getApplicationSummary(app: string, entities = ['resources']): IPromise<IManagedApplicationSummary<'resources'>> {
// ...
}
public static getEnvironmentsSummary(app: string): IPromise<IManagedApplicationSummary<'resources' | 'artifacts' | 'environments'>> {
// ...
}
}
|
|
||
| export interface IManagedArtifactVersion { | ||
| version: string; | ||
| environments: IManagedArtifactVersionEnvironmentSummary[]; |
There was a problem hiding this comment.
crazy idea: what if... we just didn't give this thing a name and inlined it? Seems somewhat futile to try to name a thing that's really just a particular join on a couple other discrete things.
Can always extract it like IManagedArtifactVersion['environments'] if / when you need it on it's own.
There was a problem hiding this comment.
Yeah, I was thinking that too. Will go with that, at least for now.
| } | ||
|
|
||
| export interface IManagedEnviromentSummary { | ||
| artifacts: IManagedEnvironmentArtifact[]; |
There was a problem hiding this comment.
Similarly to IManagedArtifactVersionEnvironmentSummary, what if we just inlined this?
| return ManagedReader.getEnvironmentsSummary(application.name); | ||
| }; | ||
|
|
||
| const addEnvironments = (_application: Application, data: IManagedApplicationSummary) => { |
There was a problem hiding this comment.
Should this be IManagedApplicationEnvironmentsSummary? (or IManagedApplicationSummary<'resources' | 'artifacts' | 'environments'> if you do that thing I commented about)
| hasManagedResources: false, | ||
| environments: [], | ||
| artifacts: [], | ||
| resources: [], |
There was a problem hiding this comment.
is duplicating the resources data into this data source a temporary/convenience thing, or do you intend on it being permanent?
There was a problem hiding this comment.
Well originally temporary/convenience, but thinking about it some more I think it might be more permanent.
I don't like the duplication either but because the managedResource data source is visible: false, it doesn't contribute to the application ready state, so I don't have a guarantee that it will be available when my route loads.
|
Whoops, that merge was unintended... I was trying to update my branch with the latest master. |
Revert "feat(md): Types and new data source for environments"
|
Reverted and reopeened as #8022 |
Depends on spinnaker/keel#848
The types are probably the most interesting thing in this PR. There's a fair amount of duplication in different views of an entity, for example:
environmentsvsartifact.version.environmentsenvironments: a light summary of all resources and all artifacts related to those resourcesartifact.version.environments: a view of each environment as it relates to the artifact's specific version (i.e. time that version was deployed in that specific environment)artifactsvsenvironment.artifactsvsresource.artifactartifacts: summary of all artifacts and all versions also including a join between a specific version and it's environmentenvironment.artifacts: a view of each artifact that is in a given environment (i.e. approved, vetoed)resource.artifact: (not yet present) a view of a specific artifact as it relates to the resource (i.e. deploying, pending)The shapes or mostly different, so I'm not sure if it's even useful to have common attributes between the types. I also don't know if there's a sane way to name them.
optionalandoptInmean this datasource is only enabled if explicitly enabled.hiddenmeans this (for now) is not yet exposed in the application config UI.