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

Update parameter defaults for QuotaClient.update_quota() #489

Merged
merged 10 commits into from
Jul 27, 2024

Conversation

bernt-matthias
Copy link
Contributor

No description provided.

bioblend/_tests/TestGalaxyQuotas.py Outdated Show resolved Hide resolved
bioblend/_tests/TestGalaxyQuotas.py Outdated Show resolved Hide resolved
bioblend/_tests/TestGalaxyQuotas.py Outdated Show resolved Hide resolved
@bernt-matthias
Copy link
Contributor Author

bernt-matthias commented Jul 24, 2024

I wanted to replicate the error shown in 8f11841:

bioblend.ConnectionError: Unexpected HTTP status code: 400: {"err_msg":"Quota 'non_default_quota' is not a default.","err_code":400008}

Which happens when updating a non-default quota (created with default="no") with default="no". My intuition was that this should work. The problem occurs because the Galaxy tries to unset the default setting if a value of "no" is given

which fails if it is already a non-default quota:

https://github.com/galaxyproject/galaxy/blob/e18ba472bf941632431d387479e515e3a4cf34c7/lib/galaxy/managers/quotas.py#L197

The error message is a bit weird .. and I'm wondering if this should be fixed on bioblend side by

  • using None as default value for default
  • adding a bit of documentation

Edit: Still does not work for <21.09 (probably before galaxyproject/galaxy#11315). Wondering if we need to care about this or just disable tests for earlier releases.

@nsoranzo
Copy link
Member

Pushed to the main branch a fix for:

bioblend/galaxy/workflows/__init__.py:21: error: Unused "type: ignore" comment 
[unused-ignore]
    InputsBy = Literal["step_index|step_uuid", "step_index", "step_id", "s...
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~...

@bernt-matthias
Copy link
Contributor Author

Thanks @nsoranzo . The remaining linter error seems to depend on the answers to the above question(s).

…te_quota()`` to "="

which is the same of the Galaxy API since 21.09 and makes the
``test_update_non_default_quota`` pass for Galaxy <21.09 without
having to specify the ``operation`` parameter.
@nsoranzo
Copy link
Member

@bernt-matthias Are you happy with the changes I pushed? They align with what the Galaxy API currently does.

bioblend/_tests/TestGalaxyQuotas.py Outdated Show resolved Hide resolved
bioblend/_tests/TestGalaxyQuotas.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@bernt-matthias bernt-matthias left a comment

Choose a reason for hiding this comment

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

Thanks a lot. Lets go :)

@nsoranzo nsoranzo changed the title add test for updating nondefault quotas Update parameter defaults for QuotaClient.update_quota() Jul 27, 2024
@nsoranzo nsoranzo merged commit 5b6cb7a into galaxyproject:main Jul 27, 2024
20 checks passed
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.

2 participants