-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Emit a descriptive error when the QPY version is too new #11094
Emit a descriptive error when the QPY version is too new #11094
Conversation
This commit updates the qpy load() function to ensure we are raising a descriptive error message when a user attempts to load a qpy version that's not supported by this version of qiskit. Previously it would fail but with a hard to understand internal error message which was just confusing and not helpful. For example, when trying to load a QPY version 10 payload (the version used by Qiskit 0.45.x) with qiskit-terra 0.25.2 (which only supported up to version 9) it would raise an error message like: ``` Invalid payload format data kind 'b'p''." ``` which doesn't tell you anything meaningful unless you are intimately familiar with the QPY file format and how the load() function works. With this commit it now checks if the version number is supported immediately and if it's too new it will raise an error message that says exactly that. So in the above scenario instead of that error message it will say: ``` The QPY format version being read, 10, isn't supported by this Qiskit version. Please upgrade your version of Qiskit to load this QPY payload. ``` In addition to fix CI on the stable/0.25 branch this commit puts a hard cap on the versions we test backwards compatibility with. Previously the script ran with all tagged releases of Qiskit, but with the recent release of the 0.45.0rc1 tag we no longer can do that. The QPY format emitted by 0.45.0rc1 is not something that the 0.25.x release series can read.
One or more of the the following people are requested to review this:
|
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 mentioned on #11074 that I'm not 100% comfortable with us pushing this kind of thing to stable branches, since it can technically make scripts that were previously working completely fine break while updating to a new patch. The principle of that is more to do with our handling of QPY with regard to our stability policies, though, so not directly related to this PR.
In this case, the PR only affects a QPY loader capable of handling versions up to 9, and version 10 changes in such a way that no QPY version 10 file could ever be read by a version 9 loader, so the exception is safe to raise.
elif [[ ${parts[1]} -gt 25 ]] ; then | ||
exit 0 |
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'm fine to merge this directly to the stable/0.25
branch, since that branch is about to be staled, but for the main
version, it'd be good to do this more dynamically so it doesn't need to be hardcoded during the release process.
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.
Also technically this whole set of checkers will stop working right with the release of Qiskit 1.0, but again, that's a separate problem.
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.
Yeah, I've started to draft a PR to fix this locally. I'll push up a proper fix to main (and for stable/0.45 shortly). I did it this way for stable/0.25 in the interest of unblocking the 0.25.3 release and because we'll stop supporting the branch after 0.45.0 final is released.
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 pushed up the PR for this here: #11101
Summary
This commit updates the qpy load() function to ensure we are raising a descriptive error message when a user attempts to load a qpy version that's not supported by this version of qiskit. Previously it would fail but with a hard to understand internal error message which was just confusing and not helpful. For example, when trying to load a QPY version 10 payload (the version used by Qiskit 0.45.x) with qiskit-terra 0.25.2 (which only supported up to version 9) it would raise an error message like:
which doesn't tell you anything meaningful unless you are intimately familiar with the QPY file format and how the load() function works. With this commit it now checks if the version number is supported immediately and if it's too new it will raise an error message that says exactly that. So in the above scenario instead of that error message it will say:
In addition to fix CI on the stable/0.25 branch this commit puts a hard cap on the versions we test backwards compatibility with. Previously the script ran with all tagged releases of Qiskit, but with the recent release of the 0.45.0rc1 tag we no longer can do that. The QPY format emitted by 0.45.0rc1 is not something that the 0.25.x release series can read.
Details and comments