Skip to content

Build metrics routes#39

Merged
nbarnabee merged 8 commits intoimprove_data_fetchingfrom
build_metrics_routes
Feb 28, 2023
Merged

Build metrics routes#39
nbarnabee merged 8 commits intoimprove_data_fetchingfrom
build_metrics_routes

Conversation

@nbarnabee
Copy link
Collaborator

This PR addresses T324674

@nbarnabee
Copy link
Collaborator Author

GET requests to the various /api/metrics endpoints will now return arrays of objects, with each object containing the description of the metric (e.g., "Number of tools in the database:") and the associated value. On the frontend side, it should be extremely easy to generate information cards based on this data.

@nbarnabee
Copy link
Collaborator Author

I realized that my error handling was completely failing and have fixed it.

@nbarnabee nbarnabee force-pushed the improve_data_fetching branch from 6cb44cc to da5ef04 Compare February 27, 2023 17:04
@nbarnabee nbarnabee force-pushed the build_metrics_routes branch from 1ca9bb3 to 7623fc0 Compare February 27, 2023 17:14
@blancadesal
Copy link
Member

Could the metrics/user endpoint be folded into the metrics/contributions endpoint, passing the username as a query parameter?

@nbarnabee
Copy link
Collaborator Author

nbarnabee commented Feb 28, 2023

Could the metrics/user endpoint be folded into the metrics/contributions endpoint, passing the username as a query parameter?

Do you imagine a scenario where the user could look up any user's contributions?

I like that idea, for a future dashboard expansion, but I think I would want to make that a separate endpoint, since it would need to be tied to a form and should be displayed in its own place, without "total contributions" and "global contributions in the next 30 days".

If I folded metrics/user into metrics/contributions I'd probably just slam the two together and add an if statement -- e.g., if there's a user currently logged in, the endpoint will return both that user's contribution stats and the global ones. If there's no user currently logged in, it will return just the global stats.

How does that sound?

Move API_URL to the config file
Combine functions that communicate with Toolhub into ToolhubClient class
Create utils.py to hold ToolhubClient and other utility functions
Instantiate ToolhubClient in api/__init__.py
Modify imports accordingly
Run files through local CI
The new schema is generalized and GET requests to the /api/metrics
endpoints will now result in an array that can be easily iterated over
in order to generate tables on the dashboard.
The functions will now return the correct error message in the event
that a db connection cannot be established.  (If this passes muster I
will apply it to all of the api routes.)
Somehow managed to flub part of my manual merge when rebasing
@nbarnabee nbarnabee force-pushed the build_metrics_routes branch from 346ddbe to c7bbd71 Compare February 28, 2023 13:20
@blancadesal
Copy link
Member

Could the metrics/user endpoint be folded into the metrics/contributions endpoint, passing the username as a query parameter?
I like that idea, for a future dashboard expansion, but I think I would want to make that a separate endpoint, since it would need to be tied to a form and should be displayed in its own place, without "total contributions" and "global contributions in the next 30 days".

Ok, let's leave things as they are for now!

@nbarnabee nbarnabee merged commit 5031d29 into improve_data_fetching Feb 28, 2023
@nbarnabee nbarnabee deleted the build_metrics_routes branch February 28, 2023 13:31
blancadesal pushed a commit that referenced this pull request Mar 1, 2023
* Tidy and consolidate backend utility functions

Move API_URL to the config file
Combine functions that communicate with Toolhub into ToolhubClient class
Create utils.py to hold ToolhubClient and other utility functions
Instantiate ToolhubClient in api/__init__.py
Modify imports accordingly
Run files through local CI

* Improve api endpoint descriptions

This addresses T330126

* Make minor adjustments to docstrings

This addresses comments by blancadesal

* Add basic randomizer to /api/task GET routes

This solves the problem of Toolhunt always returning the same set of
10 tasks, though it does nothing to mitigate the possibility that
multiple users might be given the same task at a given moment.  (At
release, though, that would be very unlikely.)

* Apply CI fixes

* Decouple ToolhubClient from app context

This commit addresses requested changes.
I've also improved naming of the instantiations of ToolhubClient, and
rearranged some imports.

* Build metrics routes (#39)

* Tidy and consolidate backend utility functions

Move API_URL to the config file
Combine functions that communicate with Toolhub into ToolhubClient class
Create utils.py to hold ToolhubClient and other utility functions
Instantiate ToolhubClient in api/__init__.py
Modify imports accordingly
Run files through local CI

* Add function to generate a date X days in the past

* Add metrics schemas

* Add metrics routes and take into use

* Improve metrics schemas and /api/metrics responses

The new schema is generalized and GET requests to the /api/metrics
endpoints will now result in an array that can be easily iterated over
in order to generate tables on the dashboard.

* Fix error handling for /metrics routes

The functions will now return the correct error message in the event
that a db connection cannot be established.  (If this passes muster I
will apply it to all of the api routes.)

* Commit CI fixes

* Bugfix: remove instances of duplicated code

Somehow managed to flub part of my manual merge when rebasing

* fix: Avoid circular import issues

* Makes it possible to access current_app outside of an application
  context (see https://flask.palletsprojects.com/en/2.2.x/appcontext/)
* With this, the app config can be accessed outside of the routes,
  meaning the ToolhubClient can be instantiated outside of the
  functions that use it, and then passed as an argument. The
  advantages of doing things this way ("dependency injection") will
  hopefully become more obvious once we start writing tests.

* Improve error handling for PUT requests

---------

Co-authored-by: Slavina Stefanova <sstefanova@wikimedia.org>
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