Skip to content

base: compose-apps-early-start: avoid composectl call with empty apps list#1720

Open
MrCry0 wants to merge 1 commit into
foundriesio:mainfrom
MrCry0:cryo/fix-compose-apps-early
Open

base: compose-apps-early-start: avoid composectl call with empty apps list#1720
MrCry0 wants to merge 1 commit into
foundriesio:mainfrom
MrCry0:cryo/fix-compose-apps-early

Conversation

@MrCry0

@MrCry0 MrCry0 commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

In start_restorable_apps(), when shortlist is empty (all default apps are already present locally) and the compose-apps directory is also empty, the code fell into an else branch and executed:

composectl run --apps ""

Since shortlist is empty, there are no apps to restore. Calling composectl with an empty --apps argument is pointless and may trigger an error that fails the service.

Replace this branch with a log message to make the no-op explicit.

@angolini angolini left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe how you have tested it?

@angolini

Copy link
Copy Markdown
Contributor

@caiotpereira take a look in this PR and let's talk about dependency and testing

@angolini

Copy link
Copy Markdown
Contributor

well, this was replaced by #1719 and #1720

@angolini angolini closed this Mar 17, 2026
@quaresmajose

Copy link
Copy Markdown
Member

well, this was replaced by #1719 and #1720

There are one commit in this PR1719 that was not merged, only the one in #1719 was merged.

@angolini angolini reopened this Mar 18, 2026
@angolini

Copy link
Copy Markdown
Contributor

@quaresmajose comented that there is one commit here that is not present in the other PRs. Let's keep this PR open until @MrCry0 can review and rebase the changes

… list

In start_restorable_apps(), when shortlist is empty (all default apps
are already present locally) and the compose-apps directory is also
empty, the code fell into an else branch and executed:

  composectl run --apps ""

Since shortlist is empty, there are no apps to restore. Calling
composectl with an empty --apps argument is pointless and may trigger
an error that fails the service.

Replace this branch with a log message to make the no-op explicit.

Fixes: 0eecf72 ("base: compose-apps-early-start: update compose apps early start")
Signed-off-by: Oleksandr Suvorov <cryosay@gmail.com>
@MrCry0 MrCry0 force-pushed the cryo/fix-compose-apps-early branch from 276664a to 8b2135a Compare March 18, 2026 18:23
@MrCry0 MrCry0 changed the title base: compose-apps-early-start: fix two bugs in start_restorable_apps base: compose-apps-early-start: avoid composectl call with empty apps list Mar 18, 2026
@quaresmajose

Copy link
Copy Markdown
Member

It would be good to have the opinion of @mike-sul or @detsch.

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.

3 participants