Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 job_tags and max_execution_time to options #510
Add job_tags and max_execution_time to options #510
Changes from 9 commits
e2e3c89
756a2c0
543eefa
8efc285
15bb91f
20ed4c3
32076f3
3e2b0cc
1b55407
e3bfaef
eaeac76
5cc1fd5
a1afacf
00fad22
96226f9
2fc4e65
8c5beb7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 created a new
EnvironmentOptions
in my last PR as an attempt to organize less commonly used options (such asimage
) that is not related to transpilation or execution. I thinkjob_tags
can go there too, butmax_execution_time
can stay at level 1 as it's more likely to be 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.
This function is actually meant to return only fields inside the
RuntimeOptions
class to be passed toQiskitRuntimeService.run()
. The docstring ofQiskitRuntimeService.run()
says theoptions
parameter can only bebackend
,image
, andlog_level
. If you want to change that you'll need to update docstring + release note.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 your intent is to move
job_tags
andmax_execution_time
to be part ofRuntimeOptions
class. If that's the case, then 1) the use of them as separate input args should be deprecated and 2) docstring should list them as bullet points, similar tobackend
,image
, andlog_level
.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 something we want to do? Deprecate
job_tags
andmax_execution_time
as input args and have them passed in through inoptions
instead?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 since I think it's rather confusing to have some options inside
options
and some outside.