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 early exit upon Helm exit code 1 #376

Merged
merged 11 commits into from
Oct 23, 2023
Merged

Fix early exit upon Helm exit code 1 #376

merged 11 commits into from
Oct 23, 2023

Conversation

sujuka99
Copy link
Contributor

@sujuka99 sujuka99 commented Oct 21, 2023

fixes #373

Fix

I am getting this error when I try to use dry-run: subprocess.CalledProcessError: Command '['helm', 'get', 'manifest', 'account-producer', '--namespace', 'my-namespace']' returned non-zero exit status 1. why? does he launch it first in dry-run ? when I do that in my commandline I get
To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke
Error: release: not found

The problem came from setting subproces.run(..., check = True) without handling the raised exception.

Preventive measures

I took some imports out of TYPE_CHECKING blocks to avoid problems with pydantic as it uses type hints at runtime

Ruff

Updated to the latest (0.1.1) version. Now all fixes deemed unsafe will not be automatically carried out even if it is possible to do so. The user can choose to enable the unsafe autofixes with the --unsafe-fixes flag

Example

While testing I noticed that we could use the namespace env var in some places in the example, so I did it and had to adjust the tests.

@sujuka99 sujuka99 self-assigned this Oct 21, 2023
kpops/component_handlers/helm_wrapper/helm.py Outdated Show resolved Hide resolved
tests/pipeline/test_pipeline.py Outdated Show resolved Hide resolved
Copy link
Member

@disrupted disrupted left a comment

Choose a reason for hiding this comment

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

please keep consistent capitalisation in PR title, i.e. Helm

kpops/component_handlers/helm_wrapper/helm.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/pipeline/test_example.py Outdated Show resolved Hide resolved
@sujuka99 sujuka99 changed the title Fix early exit upon helm exit code 1 Fix early exit upon Helm exit code 1 Oct 23, 2023
@sujuka99 sujuka99 merged commit 352cf9c into main Oct 23, 2023
7 checks passed
@sujuka99 sujuka99 deleted the fix/helm-bug branch October 23, 2023 08:56
disrupted pushed a commit that referenced this pull request Oct 25, 2023
fixes #373

> I am getting this error when I try to use dry-run:
subprocess.CalledProcessError: Command '['helm', 'get', 'manifest',
'account-producer', '--namespace', 'my-namespace']' returned non-zero
exit status 1. why? does he launch it first in dry-run ? when I do that
in my commandline I get
To learn more, consult
https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke
Error: release: not found

The [problem came
from](2.0.9...2.0.10#diff-1ca7465daf9238dfe221304df636d96704ebf4935288b854a5c4c6b3ea9e3162R215)
setting `subproces.run(..., check = True)` without handling the raised
exception.

I took some imports out of `TYPE_CHECKING` blocks to avoid problems with
`pydantic` as it uses type hints at runtime

Updated to the latest (`0.1.1`) version. Now all fixes deemed unsafe
will not be automatically carried out even if it is possible to do so.
The user can choose to enable the unsafe autofixes with the
`--unsafe-fixes` flag

While testing I noticed that we could use the namespace env var in some
places in the example, so I did it and had to adjust the tests.
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.

Kpops is broken in the version 2.0.10
3 participants