serverless / compose

Orchestrate Serverless Framework in monorepos
https://serverless.com/framework/docs/guides/compose
MIT License
111 stars 15 forks source link

Introduce support for remote state with S3 #124

Closed pgrzesik closed 2 years ago

mnapoli commented 2 years ago

I pushed a few fixes, I added some types (I added them just in places it was useful to me) to help me track down why a context variable ended up undefined.

I tried running sls remove to see if it would remove the state bucket. I was surprised to see the bucket was actually being created (I'm guessing since the bucket name changed since the last POC, it didn't exist).

I had the following output:

image

If we ignore the error (I haven't looked into it yet), I'm wondering if the state bucket be created on remove if it doesn't exist? I think it might be technically hard to write the code so that it behaves differently based on the command, so we may actually ignore that idea and leave it as-is.

pgrzesik commented 2 years ago

Thanks a lot for the review @mnapoli - I'll dive deeper into the feedback and changes tomorrow.

As for creation of bucket on remove - that is an edge case - when instantiating the state the logic always ensures that the bucket exists - we might remove that logic, but then we're in this weird situation where want to remove the state but we don't know from where (the user explicitly had the remote state configured). What do you think should be the behavior there? I honestly missed that as I didn't test a situation where someone starts with local state, configures remote and runs remove as the first command

pgrzesik commented 2 years ago

One question - with which configuration did you run into the error from the image below?

nvm - I've found the problem and pushed the fix

I pushed a few fixes, I added some types (I added them just in places it was useful to me) to help me track down why a context variable ended up undefined.

I tried running sls remove to see if it would remove the state bucket. I was surprised to see the bucket was actually being created (I'm guessing since the bucket name changed since the last POC, it didn't exist).

I had the following output:

image

If we ignore the error (I haven't looked into it yet), I'm wondering if the state bucket be created on remove if it doesn't exist? I think it might be technically hard to write the code so that it behaves differently based on the command, so we may actually ignore that idea and leave it as-is.

mnapoli commented 2 years ago

Yeah, let's actually consider that an edge case 👍

pgrzesik commented 2 years ago

I'm going to hold off with merging before the AWS credentials being finalized 👍