-
Notifications
You must be signed in to change notification settings - Fork 18.9k
remove job from image_export #12596
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
remove job from image_export #12596
Conversation
api/server/server.go
Outdated
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.
Is this trick really necessary here? I think it will be enough just return error. At least it'll preserve old behavior.
graph/export.go
Outdated
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'll totally remove this line since it's not a job anymore, or maybe write another message
Signed-off-by: Simei He <hesimei@zju.edu.cn> Signed-off-by: He Simei <hesimei@zju.edu.cn>
5122621 to
30557fc
Compare
|
thanks @dalanlan |
|
looks ok to me apart from that engine needed in exportImage |
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 we just don't return error here? I'm pretty sure that you're not returning JSONError from ImageExport.
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.
Ah, I got it. But we're not doing Used trick then?
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 think it is superweird. You should write json-error directly to w if error encountered after Flushed.
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.
It's the NEW legacy of postImagesCreate.
i thought u guys were not pleasant with Flushed or Used stuff and somehow i was not able to make binary locally due to network problem so i just corrected it in your way:(
I would write a pr to correct postImagesCreate then since it is also weird when you type docker pull scratch or sth.
part of #12151
remove job from image_export
Signed-off-by: Simei He hesimei@zju.edu.cn