-
Notifications
You must be signed in to change notification settings - Fork 33
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
Custom base images for functions #1297
Conversation
@@ -0,0 +1,253 @@ | |||
{ |
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.
nit: the rest of the procedural notebooks are in docs/getting_started/experimental
, whereas the other ones here (VQE / QAOA) are more application-like ... which begs the question of whether we need to think about how to organize our documentation (i.e. should we start making sections for end users, for function providers, etc.)
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.
And the pending move to the qiskit org probably provides a good cover for a docs reorg ...
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.
Agree with you :)
I also was trying to figure out where to put those. I was trying to decide between examples
or deployment
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.
Finally moved it to deployment as rst document.
@@ -0,0 +1,253 @@ | |||
{ |
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.
Ideally we should also add this to the e2e tests too
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.
Created separate issue for that #1316
Not sure yet how to make e2e, since it will not work in local mode and in docker compose as well.
@@ -0,0 +1,7 @@ | |||
FROM icr.io/quantum-public/quantum-serverless-ray-node:0.9.0-py39 |
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 might be worth creating a template repo in the Qiskit org to store these files so folks can make use of it when writing their own functions. Also would give us a little insight into some of the third party functions out there in the wild
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.
Good suggestion! I like it! Templates in github are very powerful
Co-authored-by: Paul Schweigert <paul@paulschweigert.com>
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.
Thank you so much, @IceKhan13 I left a couple of comments but reviewing my comments + @psschwei comments is almost ready to merge 🙏
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.
Regardless @psschwei comments from my part LGTM! 🥳
Thanks @IceKhan13 !!
data={ | ||
"title": program.title, | ||
"image": program.image, | ||
"arguments": json.dumps({}), |
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.
unrelated question. Is the arguments
for the program
used?
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.
probably you are right and it is not used anymore. I will look it up and try to fix it in following PRs
f"{artifact_file_path} is {int(size_in_mb)} Mb, " | ||
f"which is greater than {MAX_ARTIFACT_FILE_SIZE_MB} allowed. " | ||
f"Try to reduce size of `working_dir`." | ||
elif program.entrypoint is not None: |
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 it good to put a waring message when both image and entrypoint are set?
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 would be better to move all validation to QiskitFunction itself to reduce boilderplate code. Created an iissue for that. #1315
Thanks for suggestion!
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.
Great work! Thanks!
Summary
Allows usage of custom images for function execution
Details
User will need to do following steps:
Function template
Dockerfile
Upload funciton with custom image