Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(app): Verify existence of review app before resizing process #77

Merged
merged 4 commits into from
Oct 9, 2024

Conversation

CuriousLearner
Copy link
Contributor

We already check that the DeployStatus d is not nil, so checking if the processes within it are loaded and raise an error from FindProcess.

https://github.com/apppackio/apppack/blob/main/cmd/ps.go#L100-L101

@ipmb
Copy link
Member

ipmb commented Sep 13, 2024

I'm not sure that the user should see an error in this scenario. If they should, that one won't make any sense to them.

The scenario is that they are trying to configure a review app before it has finished it's first deployment. The command causing the error was:

apppack -a iris:3561 ps resize web --cpu 1 --memory 2G

I can think of two possible responses here:

  1. Exit with an error, "review app does not exist"
  2. Mimic the behavior from fix(cmd/ps): resize raises warning for non-existent service #60

1 should happen if the review app stack does not exist. 2 should happen if the review app exists but the service does not.

@CuriousLearner
Copy link
Contributor Author

I discovered that the initial error happens when the reviewapp does not exists. We handle it now.

I tried to replicate scenario to resize a process while reviewapp was being created, but was able to do it successfully.

1 should happen if the review app stack does not exist. 2 should happen if the review app exists but the service does not.

Both of these happen now.

@CuriousLearner CuriousLearner changed the title fix(app): Raise error in case no processes are found in DeployStatus fix(app): Check reviewapp existence before resizing process Oct 3, 2024
@CuriousLearner CuriousLearner changed the title fix(app): Check reviewapp existence before resizing process fix(app): Verify existence of review app before resizing process Oct 3, 2024
@CuriousLearner CuriousLearner merged commit 2b2da0d into main Oct 9, 2024
3 of 4 checks passed
@CuriousLearner CuriousLearner deleted the fix-d-process-nil-issue branch October 9, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants