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

Use endpoint IDs for endpoint predict requests #78

Closed
wants to merge 1 commit into from

Conversation

phil-scale
Copy link
Contributor

Extra latency right now for sync/async requests incurred from querying for endpoint by name

@phil-scale phil-scale requested a review from a team February 15, 2023 21:59
@@ -1187,7 +1185,6 @@ def _async_request(
`abcabcab-cabc-abca-0123456789ab`
"""
validate_task_request(url=url, args=args)
endpoint = self.get_model_endpoint(endpoint_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this just a postgres call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but also Redis to get some cached data about the infra state (and k8s api server if Redis cache doesn't contain the info)

Copy link
Member

@yixu34 yixu34 left a comment

Choose a reason for hiding this comment

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

Wait is this a breaking change?

@phil-scale
Copy link
Contributor Author

Wait is this a breaking change?

Sort of? _async_request and _sync_request were technically deprecated and private anyways

@yixu34
Copy link
Member

yixu34 commented Feb 15, 2023

Wait is this a breaking change?

Sort of? _async_request and _sync_request were technically deprecated and private anyways

True, though I think enough people might have latched onto _async_request. Maybe we can limit the change to just _sync_request for now?

Maybe we can do a broader change as part of #73?

@phil-scale
Copy link
Contributor Author

Ok closing this in favor of modifying #73 instead

@phil-scale phil-scale closed this Feb 16, 2023
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.

3 participants