Skip to content

Support click 8.2 #6883

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

Draft
wants to merge 38 commits into
base: main
Choose a base branch
from
Draft

Conversation

danielhollas
Copy link
Collaborator

@danielhollas danielhollas commented May 20, 2025

There have been a huge amount of changes in 8.2.0, we might need to pin click to 8.1 until they are resolved:

https://click.palletsprojects.com/en/stable/changes/#version-8-2-0

Things that definitely affect us:

Keep stdout and stderr streams independent in CliRunner. Always collect stderr output and never raise an exception. Add a new output stream to simulate what the user sees in its terminal. Removes the mix_stderr parameter in CliRunner. #2522 #2523

In 8.2.0, just running 'verdi' or 'python -m aiida'
without any arguments returns non-zero code.
In verdi.sh we add '-h' to workaround this.
Copy link

codecov bot commented May 20, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 13 lines in your changes missing coverage. Please review.

Project coverage is 79.18%. Comparing base (cfb7fd1) to head (b66822b).

Files with missing lines Patch % Lines
src/aiida/cmdline/commands/cmd_devel.py 0.00% 6 Missing ⚠️
src/aiida/cmdline/params/types/choice.py 66.67% 4 Missing ⚠️
src/aiida/cmdline/params/types/multiple.py 83.34% 2 Missing ⚠️
src/aiida/cmdline/params/types/group.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6883      +/-   ##
==========================================
+ Coverage   79.16%   79.18%   +0.03%     
==========================================
  Files         566      567       +1     
  Lines       43489    43521      +32     
==========================================
+ Hits        34424    34459      +35     
+ Misses       9065     9062       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@danielhollas danielhollas changed the title Update click package in lockfile Support click 8.2 May 21, 2025
@danielhollas danielhollas force-pushed the lock-update-click branch 2 times, most recently from 6c1d04a to 18759ce Compare May 21, 2025 03:42
@agoscinski
Copy link
Contributor

agoscinski commented May 21, 2025

Do you know why the test-install ci workflow did not fail until the reaese 8.2.1? I was able to merge yesterday #6880 without a problem and 8.2.0 was already released.

@danielhollas
Copy link
Collaborator Author

Do you know why the test-install ci workflow did not fail until the reaese 8.2.1? I was able to merge yesterday #6880 without a problem and 8.2.0 was already released.

That's because test-install did not run on this PR. :-)

There is a nightly run of test-install that is (I am assuming) failing now, but it doesn't report anywhere so nobody noticed. Somebody should add the same Slack ping that we have for nightly tests to .github/workflows/test-install.yml as well.

@agoscinski
Copy link
Contributor

That's because test-install did not run on this PR. :-)

I see now that we specify path in the workflow. Thanks! I was a bit confused when fixing the RTD.

@agoscinski agoscinski linked an issue May 22, 2025 that may be closed by this pull request
@@ -637,22 +637,22 @@ def verify_deletion(self, nodes_deleted=True, folders_deleted=True):
if nodes_deleted:
for workflow_pk in self.workflow_pks:
with pytest.raises(NotExistent):
WorkflowNode.objects.get(pk=workflow_pk)
WorkflowNode.collection.get(pk=workflow_pk)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

/home/hollas/atmospec/aiida-core/tests/cmdline/commands/test_node.py:655: AiidaDeprecationWarning: objects property is deprecated, use collection instead. (this will be removed in v3)

@@ -372,7 +372,7 @@ def _perform_delete():
from aiida.orm.utils.remote import clean_mapping_remote_paths, get_calcjob_remote_paths
from aiida.tools.graph.graph_traversers import get_nodes_delete

backend = get_manager().get_backend()
backend = get_manager().get_profile_storage()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

/home/hollas/atmospec/aiida-core/src/aiida/manage/manager.py:251: AiidaDeprecationWarning: get_backend() is deprecated, use get_profile_storage() instead (this will be removed in v3)
warn_deprecation('get_backend() is deprecated, use get_profile_storage() instead', version=3)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After adjusting the stacklevel

/home/hollas/atmospec/aiida-core/src/aiida/cmdline/commands/cmd_node.py:375: AiidaDeprecationWarning: get_backend() is deprecated, use get_profile_storage() instead (this will be removed in v3)
backend = get_manager().get_backend()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

click 8.2.0 broke verdi
2 participants