-
-
Notifications
You must be signed in to change notification settings - Fork 626
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
CICD: Add set -e -o pipefail
to ruff
and mypy
.
#399
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes requested.
Something look wrong? You can customize Ellipsis by editing the ellipsis.yaml for this repository.
Generated with ❤️ by ellipsis.dev
@@ -36,10 +36,12 @@ jobs: | |||
python3 -m pip install -r requirements.txt | |||
- name: Run Continuous Integration Action | |||
run: | | |||
set -e -o pipefail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on adding set -e -o pipefail
to ensure the script fails if any command fails. However, please avoid suppressing linting errors using # noqa: ARG002
, # noqa: ARG003
, or # noqa: B007
in the code. Instead, please fix these linting errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SMOKED
Fixes a bug I've missed out earlier that shows failure in logs but does not fail the full CI check in GitHub action because of not adding
pipefail
. So if a chain of commandcmd1 | cmd 2 | cmd3
fails atcmd1
, the full command may still pass.Now with adding the
set -e -o pipefail
, if any command in a pipeline fails, the pipeline will return a failure status.I also synced up remote CI with local pre-commit hooks, in particular on
mypy
since @savarin is actively introducing gradual typing to the instructor library.In addition, I fixed a few small linting errors. Some are suppressed using
noqa
, for instance in line 117 inpartialjson.py
,e
is unused but was not removed to keep the interface consistent with other parsing (design choice).Summary:
This PR enhances the CI/CD pipeline by ensuring pipeline failure upon any command failure and includes minor linting fixes.
Key points:
set -e -o pipefail
tomypy
andruff
workflows in/.github/workflows/mypy.yml
and/.github/workflows/ruff.yml
./examples/partial_streaming/benchmark.py
,/instructor/dsl/iterable.py
,/instructor/dsl/partialjson.py
, and/instructor/patch.py
.Generated with ❤️ by ellipsis.dev