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

better wording when packit cannot parse YAML file #1861

Merged
merged 1 commit into from Feb 27, 2023

Conversation

xsuchy
Copy link
Contributor

@xsuchy xsuchy commented Feb 23, 2023

Previously it printed:

2023-02-23 16:23:13.305 package_config.py ERROR  Cannot load package config /tmp/fedora-upgrade/.packit.yaml.
2023-02-23 16:23:13.305 utils.py          ERROR  Cannot load package config: ScannerError('while scanning for the next token', None, "found character '\\t' that cannot start any token", <yaml.error.Mark object at 0x7f3310e2a190>).

Now it prints:

2023-02-23 17:25:59.950 package_config.py ERROR  Cannot load package config /tmp/fedora-upgrade/.packit.yaml.
2023-02-23 17:25:59.950 package_config.py ERROR    parser says
  in "<unicode string>", line 12, column 1:
              - fedora-stable
    ^
  found character '\t' that cannot start any token while scanning for the next token
2023-02-23 17:25:59.950 utils.py          ERROR  Please correct data and retry.

TODO:

  • [-] Write new tests or update the old ones to cover new functionality.
  • [-] Update doc-strings where appropriate.
  • [-] Update or write new documentation in packit/packit.dev.

RELEASE NOTES BEGIN
Command packit validate-config now provides details about errors when it cannot parse the config file.
RELEASE NOTES END

@xsuchy xsuchy force-pushed the yamlerror branch 2 times, most recently from 414da0e to 423cadb Compare February 23, 2023 16:59
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/013ca306887b498b857af79895b4563c

✔️ pre-commit SUCCESS in 2m 01s
✔️ packit-tests-rpm SUCCESS in 12m 07s
✔️ packit-tests-pip-deps SUCCESS in 12m 35s
✔️ packit-tests-git-main SUCCESS in 12m 54s
✔️ packit-tests-pip-deps-sess-rec SUCCESS in 2m 16s
✔️ packit-tests-git-main-sess-rec SUCCESS in 2m 40s
✔️ reverse-dep-packit-service-tests SUCCESS in 3m 18s

packit/config/package_config.py Outdated Show resolved Hide resolved
packit/config/package_config.py Outdated Show resolved Hide resolved
@xsuchy
Copy link
Contributor Author

xsuchy commented Feb 23, 2023

Updated.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/1ae0896a4a7e4389b00521d3ae8c3681

✔️ pre-commit SUCCESS in 1m 50s
✔️ packit-tests-rpm SUCCESS in 11m 57s
✔️ packit-tests-pip-deps SUCCESS in 12m 33s
✔️ packit-tests-git-main SUCCESS in 13m 00s
✔️ packit-tests-pip-deps-sess-rec SUCCESS in 2m 08s
✔️ packit-tests-git-main-sess-rec SUCCESS in 2m 35s
✔️ reverse-dep-packit-service-tests SUCCESS in 3m 22s

Copy link
Member

@jpopelka jpopelka left a comment

Choose a reason for hiding this comment

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

rebase and add mergeit labels once ready

EDIT: And thanks, it's definitely an improvement 👍

Comment on lines +198 to +203
msg = (
" parser says\n"
+ str(ex.problem_mark)
+ "\n "
+ str(ex.problem) # type: ignore
) # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

msg = f"  parser says\n{ex.problem_mark}\n  {ex.problem}"  # type: ignore

works OK (pre-commit doesn't complain) for me, but I don't mind having it multi-line

Copy link
Member

@TomasTomecek TomasTomecek left a comment

Choose a reason for hiding this comment

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

Thanks! I am going to update the release notes to match other: make it only a single sentence.

Previously it prints:
2023-02-23 16:23:13.305 package_config.py ERROR  Cannot load package config /tmp/fedora-upgrade/.packit.yaml.
2023-02-23 16:23:13.305 utils.py          ERROR  Cannot load package config: ScannerError('while scanning for the next token', None, "found character '\\t' that cannot start any token", <yaml.error.Mark object at 0x7f3310e2a190>).

Now it prints:
2023-02-23 17:25:59.950 package_config.py ERROR  Cannot load package config /tmp/fedora-upgrade/.packit.yaml.
2023-02-23 17:25:59.950 package_config.py ERROR    parser says
  in "<unicode string>", line 12, column 1:
              - fedora-stable
    ^
  found character '\t' that cannot start any token while scanning for the next token
2023-02-23 17:25:59.950 utils.py          ERROR  Please correct data and retry.

Waive mypy errors about
packit/config/package_config.py:198: error: "YAMLError" has no attribute "context"  [attr-defined]
packit/config/package_config.py:203: error: "YAMLError" has no attribute "problem"  [attr-defined]
packit/config/package_config.py:205: error: "YAMLError" has no attribute "context"  [attr-defined]
packit/config/package_config.py:209: error: "YAMLError" has no attribute "problem"  [attr-defined]
as they exists when the error occure.
@xsuchy xsuchy added the mergeit When set, zuul wil gate and merge the PR. label Feb 27, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/f1e3074e8ecf4b79adf5d5e37ecc27de

✔️ pre-commit SUCCESS in 1m 42s
✔️ packit-tests-rpm SUCCESS in 11m 48s
✔️ packit-tests-pip-deps SUCCESS in 12m 12s
✔️ packit-tests-git-main SUCCESS in 12m 26s
✔️ packit-tests-pip-deps-sess-rec SUCCESS in 2m 01s
✔️ packit-tests-git-main-sess-rec SUCCESS in 2m 23s
✔️ reverse-dep-packit-service-tests SUCCESS in 3m 08s

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/6767025be2094769b4dfe0f594692b60

✔️ pre-commit SUCCESS in 1m 39s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 49c3920 into packit:main Feb 27, 2023
@TomasTomecek
Copy link
Member

Well done Mirek! I already saw the first sentry issues about this and knew exactly what went wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants