Prepare FastAPI route for quotas #11315

davelopez · 2021-02-08T12:36:00Z

Ref #10889

Overview

Added some integration tests for the quotas API.
Moved the UserModel from lib/galaxy/managers/users.py to lib/galaxy/schema/schema.py.
Introduced the ModelClassField to represent a particular model_class in pydantic models. See additional comments [*].
Created pydantic models for quota operations.
Prepare the interfaces for QuotaManager and QuotasManager.
Added the FastAPI route with all endpoint operations (disabled)

Additional Comments

Some of the legacy controller actions return a raw string, which, sometimes, is a ';' separated list of messages. Should this be at least a JSON with an actual list of strings?
[*] Now most of the legacy API payloads that involve database entities have an encoded id and an additional model_class field to indicate the type of entity. After modeling every payload with pydantic models, this model_class field will be redundant? or what is the real purpose of this field?

Update

Refactor AdminActions and UsesQuotaMixin into a new QuotaManager class and remove the old mixins from QuotaAPIController and AdminGalaxy. This new QuotaManager will be used then (by composition) where needed.
Replace QuotaParamParser by the corresponding pydantic models in _schema.py
Refactor move the logic in QuotaAPIController to QuotasManager.
Move BaseAPIController.validate_in_users_and_groups to QuotasManager since it is currently only used by quotas and that way it can be shared by both controllers.
Updated legacy_expose_api, xref Migrate legacy_expose_api* decorators to expose_api* decorators #11252
In aa6249f I assumed the id must be encoded for the UserModel. This affects the api/whoami in configuration.py which is also using schema.UserModel. Hope these changes are ok @ic4f

Sorry for the long PR😅

davelopez · 2021-02-10T12:29:45Z

@jmchilton

What do you think would be the correct course of action here?

Leave this PR on hold until [WIP] Implement quota tracking options per ObjectStore. #10977 is merged and then rebase before starting to refactor the managers.
Merge this PR as it is (after fixing all checks and it is approved in review). And then opening a new one for the refactoring tasks after [WIP] Implement quota tracking options per ObjectStore. #10977 is completed.
Maybe a better approach?

mvdbeek · 2021-02-17T15:52:43Z

After modeling every payload with pydantic models, this model_class field will be redundant? or what is the real purpose of this field?

We still need this if an endpoint returns a union of something and the client needs to know what it is dealing with. A common example is the history endpoint that returns either HistoryDatasetAssociation or HistoryDatasetCollectionAssociation.

Some of the legacy controller actions return a raw string, which, sometimes, is a ';' separated list of messages. Should this be at least a JSON with an actual list of strings?

I think that's a case-by-case decision to take. If we do change this we may also have to update whatever frontend component consumes this, and depending on the context (is this likely something external scripts are consuming ?) we would need to provide a route for these legacy consumers (i.e we could add a parameter that controls the response model).

lib/galaxy/webapps/galaxy/api/quotas.py

davelopez · 2021-03-16T10:58:22Z

~~Waiting for #11904~~

- AdminActions, UsesQuotaMixin and QuotaParamParser logic moved to QuotaManager

- Add some type hints - Use fstrings - Remove unused parameters

Currently is used only for Quotas so moving it to the manager

I assumed the user.id must be encoded in the response. This affects the api/whoami in configuration.py which is the also using schema.UserModel

The util.Params object was causing problems with the pydantic deserialization.

jmchilton · 2021-09-08T13:15:17Z

Thank you - this looks really, really nice - sorry for the delay in reviewing it.

bgruening · 2021-09-08T14:08:28Z

Cool @davelopez congrats! Thanks @jmchilton for reviewing!

davelopez · 2021-09-08T14:16:31Z

No worries and many thanks @jmchilton!

Broken in galaxyproject#11315 Results in the following failure in BioBlend tests: ``` ______________________ TestGalaxyQuotas.test_create_quota ______________________ self = <bioblend._tests.TestGalaxyQuotas.TestGalaxyQuotas testMethod=test_create_quota> def test_create_quota(self): quota = self.gi.quotas.show_quota(self.quota['id']) self.assertEqual(quota['name'], self.quota_name) > self.assertEqual(quota['bytes'], 107374182400) E AssertionError: '107374182400' != 107374182400 bioblend/_tests/TestGalaxyQuotas.py:26: AssertionError ``` See https://github.com/galaxyproject/bioblend/runs/3593283826

davelopez added this to the 21.05 milestone Feb 8, 2021

davelopez added area/API area/testing/api area/testing/integration kind/enhancement kind/refactoring cleanup or refactoring of existing code, no functional changes status/WIP labels Feb 8, 2021

jdavcs mentioned this pull request Feb 8, 2021

Legacy endpoints not tested by integration tests if fastapi endpoint exists #11321

Closed

davelopez force-pushed the prepare_fastapi_route_quotas branch from 39569a7 to 27dbfd2 Compare February 10, 2021 12:06

davelopez force-pushed the prepare_fastapi_route_quotas branch from 2648e28 to a0e7be2 Compare February 12, 2021 19:28

davelopez removed the status/WIP label Feb 12, 2021

davelopez marked this pull request as ready for review February 12, 2021 23:13

jmchilton reviewed Feb 19, 2021

View reviewed changes

lib/galaxy/webapps/galaxy/api/quotas.py Outdated Show resolved Hide resolved

davelopez marked this pull request as draft February 23, 2021 14:23

davelopez force-pushed the prepare_fastapi_route_quotas branch from a0e7be2 to c0e0ca7 Compare March 1, 2021 11:44

davelopez added the status/WIP label Mar 1, 2021

davelopez force-pushed the prepare_fastapi_route_quotas branch 2 times, most recently from aa6249f to 8fcefe0 Compare March 2, 2021 14:13

davelopez removed the status/WIP label Mar 2, 2021

davelopez marked this pull request as ready for review March 2, 2021 17:02

davelopez force-pushed the prepare_fastapi_route_quotas branch from 8c5cf3d to 1825a6d Compare March 11, 2021 17:44

davelopez marked this pull request as draft March 11, 2021 18:13

bgruening mentioned this pull request Mar 15, 2021

Progressively adapt old API routes to fastAPI #10889

Open

55 tasks

davelopez mentioned this pull request Apr 13, 2021

Create central pydantic models #11827

Merged

3 tasks

davelopez modified the milestones: 21.05, 21.09 May 3, 2021

davelopez added 14 commits June 2, 2021 10:22

Fix quotas api delete tests

1478026

Refactor Mixins into Manager

5fe3454

- AdminActions, UsesQuotaMixin and QuotaParamParser logic moved to QuotaManager

Cleanup QuotaManager a bit

c80f773

- Add some type hints - Use fstrings - Remove unused parameters

Refactor QuotasController to use QuotasManager

61344d5

Remove validate_in_users_and_groups from BaseAPIController

5d3f7b8

Currently is used only for Quotas so moving it to the manager

Fix Quota description lost after rename

8e3a37e

Use encoded id in schema.UserModel

2694803

I assumed the user.id must be encoded in the response. This affects the api/whoami in configuration.py which is the also using schema.UserModel

Fix package import

df6d0a9

Use galaxy Router

6490747

Use json format in tests payloads

7fc2832

Enable FastAPI router

9fdeb07

Rename QuotasManager to QuotasService for consistency

2c4648b

Add url_for deprecation

3e2562d

Add GroupModel pydantic model to schema,py

81be6ff

davelopez force-pushed the prepare_fastapi_route_quotas branch from 2fbe1bd to 81be6ff Compare June 2, 2021 09:05

davelopez marked this pull request as draft June 2, 2021 09:18

davelopez added 2 commits June 2, 2021 13:31

Use require_admin flag in endpoints instead of dependencies property

e711d5d

Fix create quota from Admin UI controller

9a45c2a

The util.Params object was causing problems with the pydantic deserialization.

davelopez marked this pull request as ready for review June 2, 2021 13:23

jmchilton merged commit 62295ef into galaxyproject:dev Sep 8, 2021

davelopez deleted the prepare_fastapi_route_quotas branch September 8, 2021 14:15

davelopez mentioned this pull request Sep 8, 2021

Adapt quotas tests to use JSON payload #12417

Merged

3 tasks

nsoranzo mentioned this pull request Sep 15, 2021

Fix quota bytes type in pydantic #12462

Merged

5 tasks

bernt-matthias mentioned this pull request Jul 25, 2024

Update parameter defaults for QuotaClient.update_quota() galaxyproject/bioblend#489

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare FastAPI route for quotas #11315

Prepare FastAPI route for quotas #11315

davelopez commented Feb 8, 2021 •

edited

Loading

davelopez commented Feb 10, 2021

mvdbeek commented Feb 17, 2021

davelopez commented Mar 16, 2021 •

edited

Loading

jmchilton commented Sep 8, 2021

bgruening commented Sep 8, 2021

davelopez commented Sep 8, 2021

Prepare FastAPI route for quotas #11315

Prepare FastAPI route for quotas #11315

Conversation

davelopez commented Feb 8, 2021 • edited Loading

Overview

Additional Comments

Update

davelopez commented Feb 10, 2021

mvdbeek commented Feb 17, 2021

davelopez commented Mar 16, 2021 • edited Loading

jmchilton commented Sep 8, 2021

bgruening commented Sep 8, 2021

davelopez commented Sep 8, 2021

davelopez commented Feb 8, 2021 •

edited

Loading

davelopez commented Mar 16, 2021 •

edited

Loading