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

python@3.10 3.10.7 #109777

Closed

Conversation

branchvincent
Copy link
Member

@branchvincent branchvincent commented Sep 6, 2022

Created with brew bump-formula-pr.

resource blocks may require updates.

@BrewTestBot BrewTestBot added bump-formula-pr PR was created using `brew bump-formula-pr` CI-linux-self-hosted Build on Linux self-hosted runner legacy Relates to a versioned @ formula labels Sep 6, 2022
@branchvincent branchvincent added long build Set a long timeout for formula testing CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels Sep 6, 2022
@carlocab
Copy link
Member

carlocab commented Sep 7, 2022

This should be done in the next hour or so.

@carlocab carlocab removed the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Sep 7, 2022
@carlocab carlocab mentioned this pull request Sep 7, 2022
@iMichka
Copy link
Member

iMichka commented Sep 8, 2022

Fix for speedtest-cli: #110021

@iMichka
Copy link
Member

iMichka commented Sep 8, 2022

Looks like poetry did not like that update:

2022-09-07T01:36:26.2528322Z ##[group]Full test poetry output
2022-09-07T01:36:26.2529036Z �[32m==>�[0m �[1mTesting poetry�[0m
2022-09-07T01:36:26.2531618Z �[34m==>�[0m �[1m/home/linuxbrew/.linuxbrew/Cellar/poetry/1.2.0/bin/poetry --version�[0m
2022-09-07T01:36:26.2532183Z �[34m==>�[0m �[1m/home/linuxbrew/.linuxbrew/Cellar/poetry/1.2.0/bin/poetry new homebrew�[0m
2022-09-07T01:36:26.2532892Z �[34m==>�[0m �[1m/home/linuxbrew/.linuxbrew/Cellar/poetry/1.2.0/bin/poetry config virtualenvs.in-project true�[0m
2022-09-07T01:36:26.2533599Z �[34m==>�[0m �[1m/home/linuxbrew/.linuxbrew/Cellar/poetry/1.2.0/bin/poetry add requests�[0m
2022-09-07T01:36:26.2534260Z Creating virtualenv homebrew in /tmp/poetry-test-20220907-1029002-1fmn9m/homebrew/.venv
2022-09-07T01:36:26.2534481Z 
2022-09-07T01:36:26.2534581Z Failed to unlock the item!

@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. To keep this pull request open, add a help wanted or in progress label.

@github-actions github-actions bot added the stale No recent activity label Sep 10, 2022
@carlocab carlocab added help wanted Task(s) needing PRs from the community or maintainers and removed stale No recent activity labels Sep 11, 2022
@xxyzz
Copy link
Contributor

xxyzz commented Sep 12, 2022

Looks like it's keyring's error, it can be disabled by setting this PYTHON_KEYRING_BACKEND=keyring.backends.null.Keyring.

@xxyzz xxyzz mentioned this pull request Sep 12, 2022
6 tasks
This was referenced Sep 12, 2022
@carlocab
Copy link
Member

poetry might have been fixed by #110433.

It would be good to rebase and re-run this. Not sure there's enough time for that before tomorrow's maintenance window though.

@cho-m cho-m added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Sep 14, 2022
@xxyzz
Copy link
Contributor

xxyzz commented Sep 15, 2022

The poetry error is not fixed, maybe keyring is still need to be disabled. I wonder why the poetry tests only fail in this pull request.

@carlocab
Copy link
Member

If we disable keyring in the test, does that mean users need to disable it on their own to use poetry?

If so, then that's not a good solution, because we are then shipping something effectively broken.

@xxyzz
Copy link
Contributor

xxyzz commented Sep 15, 2022

The test failed because Keychain can't be unlocked in CI, normal user probably won't encounter this error. But these test commands shouldn't need to find password in Keychain.

@carlocab
Copy link
Member

The test failed because Keychain can't be unlocked in CI

This was always the case, though. Why didn't it fail previously, and why is it failing now with this version of Python?

@xxyzz
Copy link
Contributor

xxyzz commented Sep 15, 2022

Maybe because it's a new feature: python-poetry/poetry#1917 (comment)

I don't know why poetry wants to use keyring in this case but other people in that issue solved this error by disabling keyring.

@carlocab
Copy link
Member

Maybe because it's a new feature: python-poetry/poetry#1917 (comment)

I don't know why poetry wants to use keyring in this case but other people in that issue solved this error by disabling keyring.

But the feature seems to be there already, but the failure appears to be triggered only by Python 3.10.7 and not Python 3.10.6. So I don't think the feature being new is enough of an explanation.

Hey @neersighted, apologies for the tag but we could use your expertise here. Does it make sense to disable keyring when testing poetry in CI here?

This makes sense to do if the keyring failure is peculiar to our CI environment. However, if it's an error a user will see when using it on their machine, then we'll want to do something else.

@carlocab carlocab removed the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Sep 15, 2022
@neersighted
Copy link
Contributor

Hi @carlocab -- keyring usage in Poetry is new, and the matrix of possible OS versions/keyring backends/local system configration is vast and well beyond the ability of any project to test. It's been pretty clear since this feature has made it out into the wild that Poetry needs more robust error handling/usage of keyring.

There should be improvements to keyring usage coming to future releases (including patch releases) of Poetry, but in the here and now, keyring should not be something you need in CI and can be safely disabled. It is a very desirable feature for end users (e.g. on macOS, storing credentials in the keychain instead of plaintext on disk), as PyPI credentials are incredibly valuable as supply chain attacks can be conducted using them.

I would suggest keeping an eye on the Poetry changelog for improvements to our handling of these errors, and in the mean time just setting the env var (or using the config file as described in python-poetry/poetry#1917) to disable keyring in CI.

@danielnachun danielnachun mentioned this pull request Sep 17, 2022
6 tasks
@chenrui333 chenrui333 mentioned this pull request Sep 18, 2022
@iMichka
Copy link
Member

iMichka commented Sep 26, 2022

Let's disable the test then: #111744

@iMichka
Copy link
Member

iMichka commented Sep 26, 2022

Fix for audio: #111745

@cho-m cho-m mentioned this pull request Sep 28, 2022
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

I've seen the macOS failures elsewhere so they're pre-existing. Having python3 built against gcc-5 is also causing failures elsewhere on Linux, so let's merge this.

We probably want to set CC=cc on Linux or something to avoid hardcoding the GCC version.

@BrewTestBot
Copy link
Member

:shipit: @carlocab has triggered a merge.

@miles170 miles170 mentioned this pull request Oct 2, 2022
6 tasks
carlocab added a commit to carlocab/homebrew-core that referenced this pull request Oct 2, 2022
Let's check that this is working fine after Homebrew#109777.
@carlocab carlocab mentioned this pull request Oct 2, 2022
@branchvincent branchvincent deleted the bump-python@3.10-3.10.7 branch October 10, 2022 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump-formula-pr PR was created using `brew bump-formula-pr` CI-linux-self-hosted Build on Linux self-hosted runner CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. help wanted Task(s) needing PRs from the community or maintainers legacy Relates to a versioned @ formula long build Set a long timeout for formula testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants