-
Notifications
You must be signed in to change notification settings - Fork 65
Switch test run to ai-json parsed set of applications #3367
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
Switch test run to ai-json parsed set of applications #3367
Conversation
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.
See comments
I'm not sure I understand what this PR is about? What is in the |
All of the available inference servers, models and demo apps |
This just creates objects with models, services, etc. It does not yet uses them.? |
I'm with @odockal on this, the data doesn't seem to be getting used for anything currently. Also, what is the end goal of the data that will be read from the file? What should it be used for? |
f9e9224
to
b27dfd2
Compare
good catch, not sure how, but it slipped out when I was rewriting it from a diff |
bca3d93
to
7b50dec
Compare
7b50dec
to
6ed26ef
Compare
6ed26ef
to
f952429
Compare
242e3f6
to
84b1845
Compare
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.
I do not have review like itself atm. It is a breaking change, we have discussed maybe even better approach to start with models first aside of running recipes which "contains" almost identical e2e functionality we might also cover individually (download model + test service is also done when running whole recipe, etc).
I am also waiting for PR check to pass reliably.
BUT. It is quite great to see the amount of tests now being run vs. before
Let's proceed with the PR. and talk about possible follow ups.
e810ff8
to
805007b
Compare
Signed-off-by: Tibor Dancs <[email protected]>
805007b
to
2e40274
Compare
@odockal fully passing now, runtime 34 minutes pr-check |
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.
LGTM, I can't find anything to improve. My test run on a windows machine timed out when trying to download a model, but I see that you managed to run them properly on the CI, so I suppose it's some setup issue on my end.
One tiny nitpick, any reason not to use states.ts from the PD core? Not sure if it's possible to do it from an extension repo, but I think it should.
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.
Nice work! Id like to be a part of the conversation for enhancing the ai lab tests as well
Model download speeds can fluctuate a lot indeed, it's just something that seems to be a limitation of huggingface. |
What does this PR do?
Screenshot / video of UI
What issues does this PR fix or reference?
Closes #3333
How to test this PR?