-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
SDK - Compiler - Stopped adding mlpipeline artifacts to every compiled template #2046
SDK - Compiler - Stopped adding mlpipeline artifacts to every compiled template #2046
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.
Hi Alexey, thank you for pinging me.
The fact that these two artifacts exist the way they do is problematic and has to be fixed, indeed.
But I'd suggest putting effort on fixing this once and for all. That means:
- Step 1: Add artifact support for components
- Step 2: Make this PR change
It will take some time, but hey, it's already been there some time and you have achieved to tackle most probably any problem that has arisen (e.g. #1289).
I can contribute to making this change, but I find myself pretty confused with how components and task factories work (in fact I've been trying to understand them lately). We can cooperate on that if it suits.
I have a plan for this, but it's a bit different. I want his PR to go first so that the changes are split into smaller PRs. The plan is to make make all component outputs usable as both parameters and artifacts. The component should not be responsible for deciding how its outputs will be used. Only the consumer decides how to consume the data - via a file or via a command-line argument. |
…facts-to-every-compiled-template
/retest |
efc9fed
to
bc48323
Compare
/lgtm |
/approve |
The sample will still work fine as it is now. I'll add the change to that file as a separate PR.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Ark-kun 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 |
/lgtm |
/retest |
1 similar comment
/retest |
Over the life of the Pipelines project we have experienced numerous issues and problems stemming from the auto-added artifacts.
This PR makes a small change to the compiler so that those artifacts are no longer auto-added to every single pipeline step. (All components that actually generate those artifacts continue to work as before.)
Fixes #1421
Fixes #1422
The compiler changes are in the following commit: bdd43a5
This change is