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

[API] Use a common dependency in model monitoring endpoints #5296

Merged

Conversation

jond01
Copy link
Member

@jond01 jond01 commented Mar 17, 2024

A mini-PR related to ML-5865 to allow neat code reuse.

Aggregate the following in common params:

  • project
  • auth_info (sub-dep)
  • db_session (sub-dep)

For reference, see:
https://fastapi.tiangolo.com/tutorial/dependencies/
https://fastapi.tiangolo.com/tutorial/dependencies/sub-dependencies/

I verified that enable_model_monitoring works.

@jond01 jond01 changed the title [API] Use common dependency in model monitoring endpoints [API] Use a common dependency in model monitoring endpoints Mar 17, 2024
@jond01 jond01 requested a review from assaf758 March 18, 2024 08:02
Copy link
Member

@Eyal-Danieli Eyal-Danieli left a comment

Choose a reason for hiding this comment

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

Cool use of that common dependency approach for the APIs :)
As a future optimization, I suggest that we will modify the _common_parameters func and use it across other MLRun APIs.

@assaf758 assaf758 merged commit b1e5459 into mlrun:development Mar 19, 2024
14 checks passed
@jond01 jond01 deleted the task/use-common-depencency-in-mm-ep branch March 19, 2024 09:11
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.

None yet

3 participants