abra app cp with chaos flag #637

New Issue

moritz · 2024-09-03T15:54:47Z

moritz commented

2024-09-03 15:54:47 +00:00

When working on recipes I often copy files to the container to test some changes without completely redeploying this app. But unfortunately abra app cp always tries to checkout to the current recipe version, so it fails if there are uncommitted changes. Adding a --chaos flag would solve this issue.
Tested version: 04aec8232f

When working on recipes I often copy files to the container to test some changes without completely redeploying this app. But unfortunately `abra app cp` always tries to checkout to the current recipe version, so it fails if there are uncommitted changes. Adding a `--chaos` flag would solve this issue. Tested version: https://git.coopcloud.tech/coop-cloud/abra/commit/04aec8232f7dc44698bbca5535d01afdc9d94a4b

👍 1

moritz added the

enhancement

label 2024-09-03 15:54:47 +00:00

decentral1se added the

abra

label 2024-09-03 21:40:29 +00:00

decentral1se added the

good first issue

label 2024-11-26 11:03:53 +00:00

ammaratef45 commented

2024-11-29 19:36:50 +00:00

Huh, I tested this and the behavior is different (it worked with my unstaged changes without trying to checkout current recipe version)

abra -v                                                                                                                                               ─╯
abra version 0.9.0-beta-e3a0af5

Huh, I tested this and the behavior is different (it worked with my unstaged changes without trying to checkout current recipe version) ``` abra -v ─╯ abra version 0.9.0-beta-e3a0af5 ```

decentral1se commented

2024-11-30 11:17:41 +00:00

Thanks for checking this!

Hmmmm looks a bit suspcious there in the code:

bba1640913/cli/app/cp.go (L47-L49)

There's no --chaos flag supplied to the command?

There has been a really large churn in the CLI / flag handling and also a large refactor in how these Git operations get performed, so plenty of space for bugs to creep in.

I'd say we want to pass the --chaos flag in this case? And perhaps we want to put down an integration test on this to ensure we don't have a regression? I see no --chaos related tests in tests/integration/app_cp.bats (docs here on how to run / write tests. it's 100% bash energy!)

Thanks for checking this! Hmmmm looks a bit suspcious there in the code: https://git.coopcloud.tech/coop-cloud/abra/src/commit/bba1640913d4fe1f8b04d9ca606d9f967a9352b4/cli/app/cp.go#L47-L49 There's no `--chaos` flag supplied to the command? There has been a really large churn in the CLI / flag handling and also a large refactor in how these Git operations get performed, so plenty of space for bugs to creep in. I'd say we want to pass the `--chaos` flag in this case? And perhaps we want to put down an integration test on this to ensure we don't have a regression? I see no `--chaos` related tests in `tests/integration/app_cp.bats` ([docs here](https://docs.coopcloud.tech/abra/hack/#running-them-locally) on how to run / write tests. it's 100% bash energy!)

ammaratef45 commented

2024-11-30 22:17:11 +00:00

This comment will be confusing if I don't give names to things so I will start with that

Tested Version: the version @moritz tested and cut the ticket based on.
My version: The abra version I tested before my response, and it is 0.9.0-beta-e3a0af5
Live version: the version @decentral1se provided a snippet from in the previous comment

Well, silly me, I didn't think about the difference of versions

My version doesn't have any mention of internal.Chaos while tested versions had the snippet below

04aec8232f/cli/app/cp.go (L53-L55)

Tested version also had this snippet
04aec8232f/cli/app/cp.go (L32-L35)

Live version doesn't have flags, did we change how flags are set for commands @decentral1se?

This comment will be confusing if I don't give names to things so I will start with that Tested Version: the version @moritz tested and cut the ticket based on. My version: The abra version I tested before my response, and it is `0.9.0-beta-e3a0af5` Live version: the version @decentral1se provided a snippet from in the previous comment -------- Well, silly me, I didn't think about the difference of versions My version doesn't have any mention of `internal.Chaos` while tested versions had the snippet below https://git.coopcloud.tech/coop-cloud/abra/src/commit/04aec8232f7dc44698bbca5535d01afdc9d94a4b/cli/app/cp.go#L53-L55 Tested version also had this snippet https://git.coopcloud.tech/coop-cloud/abra/src/commit/04aec8232f7dc44698bbca5535d01afdc9d94a4b/cli/app/cp.go#L32-L35 Live version doesn't have flags, did we change how flags are set for commands @decentral1se?

decentral1se commented

2024-12-02 00:06:40 +00:00

Live version doesn't have flags, did we change how flags are set for commands @decentral1se?

@ammaratef45 That'll be coop-cloud/abra#435 (large refactor, testing still WIP). If you check the other commands, they do have flags. You need to set --chaos on the specific command now vs. before it was everywhere (which was also a hack 🙃). We've had a long standing messy setup with the CLI lib and we're maybe not 100% out of it yet! Thanks for taking a deeper look!

> Live version doesn't have flags, did we change how flags are set for commands @decentral1se? @ammaratef45 That'll be https://git.coopcloud.tech/coop-cloud/abra/pulls/435 (large refactor, testing still WIP). If you check the other commands, they do have flags. You need to set `--chaos` on the specific command now vs. before it was everywhere (which was also a hack 🙃). We've had a long standing messy setup with the CLI lib and we're maybe not 100% out of it yet! Thanks for taking a deeper look!

~~decentral1se referenced this issue 2024-12-27 21:49:24 +00:00~~

`abra` v0.10.x-beta release planning #583

decentral1se commented

2024-12-28 20:03:17 +00:00

a0da5299fe/cli/app/cp.go (L375-L383)

https://git.coopcloud.tech/coop-cloud/abra/src/commit/a0da5299feb141e043b3060f1ec1590a9898d442/cli/app/cp.go#L375-L383

decentral1se closed this issue

2024-12-28 20:03:17 +00:00

Sign in to join this conversation.