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

add Function.get_jobs() #1406

Merged
merged 38 commits into from
Jul 25, 2024
Merged

add Function.get_jobs() #1406

merged 38 commits into from
Jul 25, 2024

Conversation

akihikokuroda
Copy link
Collaborator

Summary

This the second part of #1400

Details and comments

This PR providers Function.get_logs() method that returns the job ids of jobs that executed the function.

@psschwei
Copy link
Collaborator

The running_programs_using_decorators.py test is failing in kubernetes -- don't remember, is that one broken in general?

@Tansito
Copy link
Member

Tansito commented Jul 12, 2024

The running_programs_using_decorators.py test is failing in kubernetes -- don't remember, is that one broken in general?

It should be working from what I remember, yep

@akihikokuroda
Copy link
Collaborator Author

It's a cloudpickle one. It must get broken when gateway moved to 3.11.

@Tansito
Copy link
Member

Tansito commented Jul 12, 2024

It's a cloudpickle one. It must get broken when gateway moved to 3.11.

Oh, maybe. Maybe we can fix it if we move everything to python 3.11.

@psschwei
Copy link
Collaborator

yeah, it's been failing for a couple of weeks now (this was as far back as I looked, but that was the end of the 1st page results)

gateway/tests/api/test_v1_program.py Outdated Show resolved Hide resolved
gateway/api/views.py Show resolved Hide resolved
gateway/api/views.py Show resolved Hide resolved
@Tansito
Copy link
Member

Tansito commented Jul 19, 2024

Can we add a test here too testing the client, Aki? https://github.com/Qiskit/qiskit-serverless/tree/main/tests/basic

@akihikokuroda akihikokuroda marked this pull request as draft July 22, 2024 14:21
@akihikokuroda
Copy link
Collaborator Author

@Tansito Thanks for your review. I updated according to your comments. I couldn't make the function test in Test QS on Kubernetes because the resource available in the Github Action is not enough to build the all necessary container images. The function test is added in the Docker notebook tests.

charts/qiskit-serverless/charts/gateway/values.yaml Outdated Show resolved Hide resolved
docker-compose.yaml Outdated Show resolved Hide resolved
.github/workflows/kubernetes-deploy.yaml Outdated Show resolved Hide resolved
@Tansito
Copy link
Member

Tansito commented Jul 24, 2024

@akihikokuroda I saw that you were making changes, let me know if you want that I review something 👍

@akihikokuroda akihikokuroda requested a review from Tansito July 24, 2024 14:34
@akihikokuroda
Copy link
Collaborator Author

@Tansito Please review the changes. Thanks!

Copy link
Collaborator

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

couple of very minor things, but otherwise LGTM

.github/workflows/kubernetes-deploy.yaml Show resolved Hide resolved
tests/basic/function/Sample-Docker Show resolved Hide resolved
@akihikokuroda
Copy link
Collaborator Author

Are there any other changes required? @Tansito @psschwei Thanks!

@Tansito
Copy link
Member

Tansito commented Jul 25, 2024

Sorry @akihikokuroda , I was turning off fires today and I couldn't have more time to see this. I will try to take a final look before finish the day 🙏

Copy link
Member

@Tansito Tansito left a comment

Choose a reason for hiding this comment

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

LGTM everything, Aki. Thank you!

@akihikokuroda akihikokuroda merged commit 3c1fff4 into Qiskit:main Jul 25, 2024
18 checks passed
@akihikokuroda akihikokuroda deleted the functionjobs branch July 25, 2024 20:30
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.

3 participants