-
Notifications
You must be signed in to change notification settings - Fork 12
Support launching graph store jobs from trainer and inferencer #390
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
Conversation
|
/unit_test |
|
/integration_test |
GiGL Automation@ 19:43:39UTC : 🔄 @ 21:04:31UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 19:43:50UTC : 🔄 @ 20:50:33UTC : ✅ Workflow completed successfully. |
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.
The hope for this file in gigl/common was to operate at a layer above business logic gigl/src/*.
i.e. it can plug and play anywhere, and not reliant / specific to GiGL.
Let's continue to adopt seperation of concerns here i.e. AppliedTaskIdentifier, knowledge of GiGL components, and resource config should not live inside vertex_ai service, and the callers should have logic on how to compose the objects needed to initialize the relevant vertex ai jobs - like they do when calling existing methods for the vertex ai service.
|
Sure, updated to not rely on I know we talked about creating some new file offline but I think creating some new |
python/tests/unit/src/common/translators/vertex_ai_job_translator_test.py
Outdated
Show resolved
Hide resolved
python/tests/unit/src/common/translators/vertex_ai_job_translator_test.py
Outdated
Show resolved
Hide resolved
mkolodner-sc
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 Kyle! Did a quicker pass here since this has already been approved, a few small comments but generally LGTM once those are addressed/answered
|
/unit_test |
|
/e2e_test |
GiGL Automation@ 22:31:48UTC : 🔄 @ 23:36:07UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 22:31:50UTC : 🔄 @ 22:39:40UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 22:31:54UTC : 🔄 @ 23:52:12UTC : ✅ Workflow completed successfully. |
|
/unit_test |
|
/e2e_test |
GiGL Automation@ 22:40:20UTC : 🔄 @ 23:41:22UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 22:40:24UTC : 🔄 @ 22:50:34UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 22:40:28UTC : 🔄 @ 23:55:15UTC : ✅ Workflow completed successfully. |
mkolodner-sc
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 a lot Kyle!
Scope of work done
Support launching graph store jobs from trainer and inferencer
Also break out some shared code into utils :)
No tests since we'll have some bigger e2e tests later.
Where is the documentation for this feature?: N/A
Did you add automated tests or write a test plan?
Updated Changelog.md? NO
Ready for code review?: NO