Skip to content

Conversation

@whym
Copy link
Contributor

@whym whym commented Jun 28, 2025

A YAML parse error is not caused by the podman-compose code, and the stack trace will not be helpful
in a typical use case. Adding --verbose will still show it, for those who need it.

Fixes #1139

Some questions I have:

  • I chose log.fatal() for the parse error message that is always shown. Is it a good choice? Some parts of the code use it, others use sys.stderr.write(), for things like this.
  • I don't know if we should let it die in resolve_extends() or try to catch error and return {} in that case. I left the behavior unchanged for now.

EDIT: wrong issue number

@whym whym force-pushed the yaml-parse-error branch 4 times, most recently from 9830e4a to a2709e9 Compare June 28, 2025 05:52


def bad_compose_yaml_path() -> str:
""" "Returns the path to the bad compose file"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

obvious comment -> remove



def good_compose_yaml_path() -> str:
""" "Returns the path to the good compose file"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

obvious comment -> remove

@p12tic
Copy link
Collaborator

p12tic commented Jun 28, 2025

I chose log.fatal() for the parse error message that is always shown. Is it a good choice? Some parts of the code use it, others use sys.stderr.write(), for things like this.

I think this is historical artifact. I would prefer consistent use of log.fatal()

I don't know if we should let it die in resolve_extends() or try to catch error and return {} in that case. I left the behavior unchanged for now.

I think using sys.exit() is bad practice. If you want to spend time on fixing this, let's do it in another PR.

Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

Some nits, also please add a newsfragment.

Fixes containers#1139

Signed-off-by: Yusuke Matsubara <whym@whym.org>
@p12tic p12tic force-pushed the yaml-parse-error branch from 75899a7 to 764efd3 Compare June 30, 2025 12:33
@p12tic
Copy link
Collaborator

p12tic commented Jun 30, 2025

Rebased and squashed related commits.

@p12tic p12tic merged commit 9fe6e7f into containers:main Jun 30, 2025
8 checks passed
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.

compose.yml parser issue

2 participants