-
Notifications
You must be signed in to change notification settings - Fork 206
chore(conformance): Add timeout configuration #795
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
chore(conformance): Add timeout configuration #795
Conversation
|
Hi @SinaChavoshi. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
/ok-to-test |
0af3cb6 to
0052a23
Compare
|
@SinaChavoshi can you help me understand why the
|
That is a fair point, I was setting up the tests based on the proposed test list here , though I completely agree that inference pool accepted and lifecycle are redundant. I will remove the lifecycle and limit the PR to refactoring to resolve TODOs. The follow up PRs that I have queued up ( #807 - HTTPRoute status with non-matching InferencePool & #815 - InferencePool status with non-existent EPP service ) are more inline with 1 & 2, I will add specific coverage for 1 & 2 as you mentioned above as well. Added additional test cases 5 & 6 in proposal to cover the two cases mentioned above. Also note specifically for #807 currently inference pool does not publish a status directly see - |
|
Update the PR title and description as it no longer contains the new lifecycle test addition. The PR is now limited to refactoring the code to add timeout configuration ( to resolve TODO from previous PR ) |
robscott
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @SinaChavoshi!
This LGTM other than one tiny nit.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kfswain, robscott, SinaChavoshi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* Add inferencepool_lifecycle test. * Resolve setup issues and enable InferencePool test * removed todo comment in helper.go * Add InferencePoolLifecycle test * update comments in helper.go * remove Conformanc.go from log message * Remove lifecycle test. * Removed unused helper methods ( inference pool must have selector & must be deleted) * Set timeout values as constant * change timeout.go to timing.go
* Add inferencepool_lifecycle test. * Resolve setup issues and enable InferencePool test * removed todo comment in helper.go * Add InferencePoolLifecycle test * update comments in helper.go * remove Conformanc.go from log message * Remove lifecycle test. * Removed unused helper methods ( inference pool must have selector & must be deleted) * Set timeout values as constant * change timeout.go to timing.go
* Add inferencepool_lifecycle test. * Resolve setup issues and enable InferencePool test * removed todo comment in helper.go * Add InferencePoolLifecycle test * update comments in helper.go * remove Conformanc.go from log message * Remove lifecycle test. * Removed unused helper methods ( inference pool must have selector & must be deleted) * Set timeout values as constant * change timeout.go to timing.go
* Add inferencepool_lifecycle test. * Resolve setup issues and enable InferencePool test * removed todo comment in helper.go * Add InferencePoolLifecycle test * update comments in helper.go * remove Conformanc.go from log message * Remove lifecycle test. * Removed unused helper methods ( inference pool must have selector & must be deleted) * Set timeout values as constant * change timeout.go to timing.go
Refactor the code to add specific timeout configuration for Inference Extension.
Adding local run results with cd042fe: