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.

Sign up for GitHub

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

Jump to bottom

Migrate run end-point and use function.run instead of client.run #1320

Merged
merged 28 commits into from
May 9, 2024
Merged

Migrate run end-point and use function.run instead of client.run #1320

merged 28 commits into from
May 9, 2024

Conversation

Tansito
Copy link
Member

@Tansito Tansito commented May 8, 2024 •
edited
Loading

Summary

Close #1321, #1323

This PR covers two issues:

  • Promote run_existing to run as standard way to run Qiskit Functions
  • Migrated all the tests to the new interface taking advantage of the refactorization

Details and comments

  • Gateway:
    • Removed old run references and promoted run_existing to run end-point.
  • Client:
    • Deprecate old run_existing and updated run methods
    • Fixed list:get_programs method
    • Updated LocalClient to support list method
  • Tests: updated all the tests with the new interfaces

@Tansito Tansito mentioned this pull request May 8, 2024
Closed
@Tansito Tansito marked this pull request as ready for review May 9, 2024 00:45
@Tansito Tansito changed the title 🏗️ Remove run existing end-point Remove run existing end-point and updated tests May 9, 2024
@Tansito Tansito requested a review from akihikokuroda May 9, 2024 00:45
@Tansito Tansito requested review from psschwei, IceKhan13 and akihikokuroda and removed request for akihikokuroda May 9, 2024 00:45
IceKhan13
IceKhan13 approved these changes May 9, 2024
View reviewed changes
Copy link
Member

@IceKhan13 IceKhan13 left a comment

Choose a reason for hiding this comment

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

Ah, so much better!
Now we have one UX flow to upload and run. Simplify handling of request and infrastructure a lot!

Thank you @Tansito !

@psschwei
Copy link
Collaborator

psschwei commented May 9, 2024

This is less about the changes and more just a nit on the PR text (which will become the commit message): can we also capture that we're moving from client.run to function.run in there? Will be helpful later when searching changes locally.

@Tansito Tansito changed the title Remove run existing end-point and updated tests Migrate run end-point and use function.run instead of client.run May 9, 2024
@Tansito
Copy link
Member Author

Tansito commented May 9, 2024

How about this @psschwei ?

@psschwei
Copy link
Collaborator

psschwei commented May 9, 2024

that works

@psschwei
Copy link
Collaborator

psschwei commented May 9, 2024

LGTM pending resolution of Aki's comment

@Tansito Tansito requested a review from akihikokuroda May 9, 2024 15:39
@Tansito Tansito merged commit 641da7b into main May 9, 2024
15 checks passed
@Tansito Tansito deleted the remove-run-existing branch May 9, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akihikokuroda akihikokuroda akihikokuroda approved these changes

@IceKhan13 IceKhan13 IceKhan13 approved these changes

@psschwei psschwei Awaiting requested review from psschwei

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

/tests/basic and /tests/experimental are not updated
4 participants
@Tansito @psschwei @IceKhan13 @akihikokuroda