Skip to content
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

pass job_tags and session_id kwargs correctly #358

Merged
merged 4 commits into from Dec 14, 2023
Merged

Conversation

lillian542
Copy link
Contributor

@lillian542 lillian542 commented Dec 13, 2023

A user on the forum was trying to pass job_tags and session_id to the circuit_runner device, unsuccessfully.

They were being included in generic runtime kwargs and passed to the runtime service in the program_inputs argument, which doesn't work; job_tags belongs in the options dictionary, and session_id is its own kwarg on runtime_service.run. Now they will be passed to the correct places.

Copy link

codecov bot commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (55f5df4) 100.00% compared to head (4629b63) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #358   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          322       322           
=========================================
  Hits           322       322           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lillian542 lillian542 marked this pull request as ready for review December 13, 2023 17:02
@lillian542 lillian542 requested a review from a team December 13, 2023 17:03
@lillian542
Copy link
Contributor Author

[sc-52032]

Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't matter too much but is it possible to add testing for this?

@lillian542
Copy link
Contributor Author

Maybe if I try hard enough, but it looks like currently we don't test that kwargs work in the tests, just that passing them to the device doesn't cause the circuit to fail, so it would be a bit unprecedented.

I tried getting the information from dev._current_job, because that would be a quick and easy addition to the tests, but it doesn't include this in the metadata, so I'm not sure where/if I can access it, or if its just stored in their database for task retrieval.

Here's a screenshot where you can see that my tag ("now with a session") and session id were registered when the task ran.

image

Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking! Looks good. Here's a 🚀

Copy link
Contributor

@timmysilv timmysilv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@lillian542 lillian542 merged commit 334934e into master Dec 14, 2023
10 checks passed
@lillian542 lillian542 deleted the job-tag-fix branch December 14, 2023 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants