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: allow incomplete specs during intermediate parsing stages #5555

Merged
merged 7 commits into from
Nov 26, 2024

Conversation

beckermr
Copy link
Contributor

@beckermr beckermr commented Nov 25, 2024

Description

This PR allows incomplete specs if they do not parse by skipping them for intermediate parsing stages. This behavior has been in the code for a while if a spec was completely specified by eg jinja2 (eg a list element - {{ mpi }}). The PR extends this behavior to any soec that does not parse during intermediate stages. For the final parsing stage, specs that do not parse will raise.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Nov 25, 2024
Copy link

codspeed-hq bot commented Nov 25, 2024

CodSpeed Performance Report

Merging #5555 will not alter performance

Comparing more-bugs-noarch-python-min (c85c2ba) with main (aa7a2b6)

Summary

✅ 5 untouched benchmarks

@beckermr beckermr changed the title fix: allow incomplete specs if undefined jinja2 fix: allow incomplete specs during intermediate parsing stages Nov 26, 2024
@beckermr beckermr marked this pull request as ready for review November 26, 2024 01:54
@beckermr beckermr requested a review from a team as a code owner November 26, 2024 01:54
pytest.fail("Undefined variable did NOT cause spec parsing error!")
else:
print("parsed OK!")
except (Exception, SystemExit):
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a more concrete exception set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not reliably no. I have already fixed a bug where when the exception handling in one part of the code changed, the test suite broke because a new kind of exception was raised in another part of the code. For now, any exception will work.

Copy link
Contributor

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

LGTM, just had a small question.

@beckermr beckermr merged commit 8d0ca34 into main Nov 26, 2024
28 checks passed
@beckermr beckermr deleted the more-bugs-noarch-python-min branch November 26, 2024 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants