Hey there again, it looks like the

status.state

for

deploy

action has changed to be

status.deployStatus

. This is a problem since we are assuming all actions have

status.state

as property. Can we name it back as it was? cc @alert-helicopter-61082 @swift-garage-61180

Let's just change it back. We can revisit later if we see a reason to.

I'm on it

That change shouldn't have changed

action.state

though. Are you sure you are using the correct field? The change was made to the detail object on the action result (to avoid confusion just like these where we have two different fields with the same name)

Ah, looks like it. The

state

field is still there at the top level, right?

action.detail.state is being used by the backend to get detailed state transitions of each action

Ah, that's a mistake. The top-level

state

field is intended for that.

the detail state won't exist for all actions (for kubernetes deploys it's the kubernetes deploy state like kubectl reports it)

Gotcha. That's the wrong field to use here.

@cold-jordan-68753 you probably want the

event.payload.state

CC @chilly-gigabyte-83853 @polite-fountain-28010

What we are wondering is: it would be fine if the

status.state

has a different type, but do all action have a

status.state

? If so, can we call it always

status.state

and not

status.deployState

,

status.state

, etc?

@chilly-gigabyte-83853 the state field exists directly on the payload, thats what we should be using

I think this should all become clear if you read the event types in Core carefully (and the docs for the

ActionState

type that's used at the top level of all the status event payloads).

I am referring to the doc here: > Action kind-specific states like those above are still in the event payloads (under status.state), but we've now also added the state: ActionStateForEvent top-level field, which has the same type for all action kinds. We don't follow this anymore. We now have payload.status.state and payload.status.deployState. I'd also argue we should use both payload.state and payload.status.state, as the latter is way more descriptive of the action (and it's nice to display "deploying" instead of "running" in the UI). Am I not making sense or nitpicking?

I think it's way nicer to see

deploying

,

fetched

,

built

etc instead of just

running

and

success

if I understood the distinction correctly from the thread. I also think it makes sense that we follow through with the alignment of property names and just call it

state

and not

deployState

just like we got rid of

deployStartedAt

etc

Yeah, definitely. I think it makes most sense for the Cloud team to decide how these things should look, and for Core to simply comply with that. These types are made purely to serve the Cloud API, after all. And the needs of Cloud (API and frontend) are best known by the folks building that product.

I mean, I can definitely see Core using whatever they want internally and just translate them to what Cloud uses before sending them over.

I think we're all very much in agreement here, no need to keep harping on it. The next steps are: 1. In Cloud: Remove the Core imports and declare the types there. That's easy since it's in one place now. 2. In Cloud: Ensure the api-types package is released. 3. In Core: Use the API client from the api-types package when posting events (and for all API calls in general). 4. Cloud & Core: Add an endpoint for checking whether the current Core version is compatible with the current Cloud version.

I am in violent agreement. We really need to be able to refactor at will on the core side, this situation is kinda silly as-is.

Severely agreeing as well.

hahaha