feat: provisioner idempotence changes #1795
Conversation
| let state = ResourceState::from_str(value)?; | ||
| Ok(state) | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: Feels like there should be a better way to do this. 🤔 Not a must.
There was a problem hiding this comment.
The better way to do it would be to use an enum on Postgres I feel like.
There was a problem hiding this comment.
Or doesn't https://users.rust-lang.org/t/sqlx-how-to-deserialize-an-enum-value/107215/4 work ? Though it has to be repeated in the query, so not ideal..
There was a problem hiding this comment.
I'll reconsider this as I implement RDS. We could use an enum, I just want to see if we need to add more states in the RDS PR first (I don't expect that, but it doesn't hurt to wait).
deployer/src/deployment/run.rs
Outdated
| @@ -549,6 +550,7 @@ async fn provision( | |||
| *bytes = serde_json::to_vec(&ShuttleResourceOutput { | |||
| output: new_secrets.clone(), | |||
| custom: shuttle_resource.custom, | |||
| state: ResourceState::Ready | |||
There was a problem hiding this comment.
Might not be worth changing this model on alpha. Consider using a Beta copy of it, but we also have time to re-think how we record resources.
There was a problem hiding this comment.
As discussed I agree that it's not ideal, since the old platform isn't concerned with resourec state, nor is the new runtime, but it's not a breaking change. So I think we can leave it like this, and see if we change it as we iterate on the resources.
Edit: it was a breaking change for new version of c-s with older versions of deployer, so I made it an option to avoid that issue.
| let state = ResourceState::from_str(value)?; | ||
| Ok(state) | ||
| } | ||
| } |
There was a problem hiding this comment.
The better way to do it would be to use an enum on Postgres I feel like.
| let state = ResourceState::from_str(value)?; | ||
| Ok(state) | ||
| } | ||
| } |
There was a problem hiding this comment.
Or doesn't https://users.rust-lang.org/t/sqlx-how-to-deserialize-an-enum-value/107215/4 work ? Though it has to be repeated in the query, so not ideal..
Description of change
This moves resourcestate to common, since the runner will also need it.
How has this been tested? (if applicable)
Tested on my stack.