-
Notifications
You must be signed in to change notification settings - Fork 158
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
Add configuration to pipeline stanza of DSC to enable InstructLab pipeline #1484
base: main
Are you sure you want to change the base?
Add configuration to pipeline stanza of DSC to enable InstructLab pipeline #1484
Conversation
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
c19f08f
to
4ec11e6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1484 +/- ##
==========================================
+ Coverage 19.96% 20.00% +0.04%
==========================================
Files 162 162
Lines 10964 10996 +32
==========================================
+ Hits 2189 2200 +11
- Misses 8541 8559 +18
- Partials 234 237 +3 ☔ View full report in Codecov by Sentry. |
controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go
Outdated
Show resolved
Hide resolved
4ec11e6
to
cb45a5e
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.
Added text suggestions.
This PR can't be merged just yet 😢Please run For more info: https://github.com/opendatahub-io/opendatahub-operator/actions/runs/12705522276 |
This PR can't be merged just yet 😢Please run For more info: https://github.com/opendatahub-io/opendatahub-operator/actions/runs/12705547559 |
9f53127
to
bd7ab11
Compare
This PR can't be merged just yet 😢Please run For more info: https://github.com/opendatahub-io/opendatahub-operator/actions/runs/12902820594 |
This PR can't be merged just yet 😢Please run For more info: https://github.com/opendatahub-io/opendatahub-operator/actions/runs/12902824301 |
c191dbb
to
8b953a6
Compare
8b953a6
to
d989deb
Compare
@HumairAK should I also change the related |
d989deb
to
b9ff986
Compare
@lburgazzoli yes I think that's good to do, to be consistent, we'll make the changes on dsp side as well |
b9ff986
to
8cc7364
Compare
so, we shall expect in opt/manifests/datasciencepipelines/base/params.env with two new entries, e.g:
precheck if KFTO is enabled or not will be done in DSPO, not ODH operator, is that correct? need to updated with new example .spec.components.datasciencepipelines.managedPipelines.instructLab |
|
||
extraParamsMap := map[string]string{ | ||
managedPipelineParamsKey: string(data), | ||
platformVersionParamsKey: rr.Release.Version.String(), |
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.
why not have the platform version set done in func (s *componentHandler) Init(_ common.Platform)
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.
done
78f6b66
to
16ad243
Compare
done |
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
The API definition looks good to me. |
Description
This PR adds an configuration option to the
DataSciencePipelines
component configuration to enabledInstructLab
pipelines:Once configured, the
PREINSTALLEDPIPELINES
property of theDataSciencePipelines
'sparams.env
file will looks like:https://issues.redhat.com/browse/RHOAIENG-16509
How Has This Been Tested?
Screenshot or short clip
Merge criteria