-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Poetry 1.3 dies when run with a TTY reporting size 0/0 #7184
Comments
A user on Discord has been debugging this as well, it appears to only happen in CircleCI when using the same base image. I've asked that user to post the debugging they've done here, but the basic thing we know right now is that Poetry is receiving a SIGINT right after printing the installer summary (determined via an strace). |
Had the same problem this morning with the latest |
I'm the user @neersighted mentioned from Discord. Here's what I've got so far: Reproduction code: https://github.com/TheKevJames/experiments/blob/3c986b0df2c2a3cfac52118daa654d00250838eb/.circleci/config.yml#L13-L57 tl;dr is python 3.11 works, 3.10 breaks, using the latest docker builds of both. resource_class seems irrelevant. Doing a pip upgrade to latest seems irrelevant. |
Also confirmed this is only ever headless runs, eg. I cannot reproduce when ssh'd in and running |
Most notably in that, I see that the SIGINT is happening in both cases -- on further inspection, I think we may have been incorrect in our first read through @neersighted : I don't think this is poetry reporting a SIGINT, but rather us cleaning up our SIGINT handler as part of normal shutdown procedures. It looks like poetry is actually self-terminating in response to the forked process exiting (with exit code 0), I think?? |
✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨ I also see this happen in python versions 3.8 to 3.10, but it doesn't happen on 3.11 (both |
Good job gathering those straces, it looks like what is happening is the worker is throwing an exception and we're swallowing it. This appears to be due to some issue getting Cleo's poetry/src/poetry/installation/executor.py Lines 276 to 297 in 128c528
I don't have more time to dig into this right now, but the exception handling here is plainly suspect (and we definitely should not be doing a conditional import in an exception handler). |
Yup, definitely a Cleo issue:
|
Ah, |
Ultimate cause is this PR: python-poetry/cleo#175 Note that the stdlib method we're using acts differently in 3.11, which masks the issue: https://docs.python.org/3/library/shutil.html#shutil.get_terminal_size |
Note also that the stdlib method falls back to using the terminal height/width values, which explains why the behaviour is different for interactive/non-interactive shells. |
Confirming this by running the following on CircleCI, in otherwise equivalent jobs:
|
I think that makes Circle unique here is the fact it's not allocating a TTY, unlike most CI systems (which at this point, tend to provide full VT emulation so they can capture nice colorful/fancy logs). This should be possible to reproduce locally with |
Yeah, it didn't even occur to me earlier on that this might be the case. Fancy logs generally show in CircleCI, so I made the erroneous assumption it was just via standard emulation. Tossed up a quick fix PR here, though having someone with more Cleo-specific experience take a look would be fantastic. I'd also note that it took patching poetry source code to discover this issue, given the cleo stuff was eating the trace. I wonder if there's a way we could at least fall back to not-so-fancy output in case of any errors? |
I don't think we should try to do anything fancy here, this is a pretty rare and pathological case that has revealed a big hole in test coverage. I would definitely drop the conditional import and reduce the complexity of what we do in the exception handler, but I don't think that bailing out of Cleo I/O is useful (especially as it's needed to test this code). |
You'd definitely have a better understanding than I on this! I was thinking potentially could be done fairly simply, though, eg. as a global try-except: except ...:
try:
# all the existing exception handler stuff, fancy formatting, whatever
except Exception:
traceback.print_exc() |
* try to use current ubuntu image in CircleCI * use no terminal in CircleCI tests for poetry see python-poetry/poetry#7184 for details of the issue. * added testrail case numbers and parametrize * removed all mark.smoke marker - not needed * added more testrail case numbers Co-authored-by: therealmarv <1050582+therealmarv@users.noreply.github.com>
* feat(common-utils): This implements a library to be leveraged for shared code. The module added for this task is for deployment of docker images and Prefect filesystems README.md has lots of details * ci(.circleci): adding the build step for common-utils * ci(.circleci): poetry does not need to be installed * style(.aws): fixing style check findings * ci(.circleci): moving common-utils build to separate workflow * ci(.circleci): setting up proper stdout and fixing black command * ci(.circleci): trying to get output for commands to troubleshoot errors * fix: fixing build by not create venv and fixing bug with cli:main where no args (--help for example) was throwing error * ci(.circleci): removing poetry run commands * ci: adding verbose logging to poetry * ci: moving build to using Prefect image since this is the environment this package will run on * Add --no-ansie flag as per python-poetry/poetry#7184 --------- Co-authored-by: Brett Kochendorfer <bkochendorfer@mozilla.com>
We've been experiencing this issue with poetry install and poetry update (and self update) on circleci. I just tested out a self update to use Poetry version 1.4, and it seems the issue still exists. (The release notes didn't indicate otherwise, but in case that confirmation is helpful) |
As a data point and for searchability, I encountered this with Poetry 1.4, but on Buildkite and not CircleCI. Same terminal issue there though:
Running |
For current status, we've got a proposed fix here from me that won't get accepted as per @neersighted 's comment and a better fix here from the cleo maintainer @Secrus , that unfortunately doesn't yet solve the problem. As I understand, we're waiting on @Secrus to fix and then merge&release that PR. Until then, all versions of Poetry v1.3+ on CircleCI and Buildkite (maybe others?) are broken for any commands not using the |
Poetry does not like it, for some reason. python-poetry/poetry#7184
Poetry does not like it, for some reason. python-poetry/poetry#7184
Airflow with DockerOperator ando option 'tty=True' seems to fail also for the same reason |
Hello. This affects us within |
I think the current solution is to just make sure you always use |
Poetry does not like it, for some reason. python-poetry/poetry#7184
Poetry does not like it, for some reason. python-poetry/poetry#7184
Poetry does not like it, for some reason. python-poetry/poetry#7184
@iainelder it was closed because the PR with the solution was merged. It will be visible after Cleo gets a new release (it's close, probably sometime in mid-July). |
|
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
-vvv
option) and have included the output below.Issue
Running as part of a circle ci workflow. Steps below. The command
poetry install
identifies thePackage Operations
and then exits with code 1. (output below)SSH into the box and running
poetry install
produces the normal expected behavior.workflow steps
###Poetry Install Step
The text was updated successfully, but these errors were encountered: