Qiskit / qiskit-serverless

A programming model for leveraging quantum and classical resources
https://qiskit.github.io/qiskit-serverless/
Apache License 2.0
68 stars 31 forks source link

Provider can retrieve a job from its function #1470

Closed Tansito closed 1 month ago

Tansito commented 2 months ago

Steps to reproduce the problem

To retrieve a specific job we have get_job_by_id. The problem is that we are limitating the retrieve of that job to the author and providers should be able to retrieve the job if it's a job from a owned function.

What is the expected behavior?

Providers should have access to all function jobs to check logs, errors and results.

akihikokuroda commented 2 months ago

The jobs methods in the QiskitFunctionCatalog and the QiskitServerless only returns the jobs created by the user. It may not make sense that the get_job_by_id returns the job that may not created the user. Do we need some set of methods that can be used by the users in the provider group?

Tansito commented 2 months ago

It may not make sense that the get_job_by_id returns the job that may not created the user

It has sense as soon as we are allowing to the providers access to logs and results from jobs not created by themselves what it has sense to debug and collect information.

Btw, they can have access using get_jobs right now (if I remember correctly this is done through function.get_jobs() or something like that). In the gateway we are returning all jobs related with a function if they are a provider of the function. The only inconsistency that it remains is this one (that doesn't mean that we don't need to review our current end-points and refactorize them later on).

Tansito commented 2 months ago

Or function.get_jobs() changes in the catalog?

akihikokuroda commented 2 months ago

You are right. No, it hasn't changed. The QiskitFunction.get_jobs returns the job ids of the function. I didn't look at the QiskitFunction methods. I only looked at QiskitServerless and QiskitFunctionCatalog. How about add job_id argument to the get_jobs method of the QiskitFunction instead of changing the get_job_by_id in QiskitServerless or QiskitFunctionCatalog? Probably change the return value of the method to the job details instead of the job id when the job_id argument is given.

Tansito commented 2 months ago

I think what you are proposing has a lot of sense but I'm afraid that having so similar apis returning different things would be confusing. Let me take a last look to the proposal interface in the issue and double check it with Sanket too, thanks Aki for the appointment 🙏

Tansito commented 2 months ago

I think I have a middle-term proposal here, @akihikokuroda . In reality all the points from Jobs use the id of the job, there is no more info required. So we could instantiate a Job in the client with that info so in that way the could call to logs, result and so on. So they could:

jobs = function.jobs() -> [Job] # (but these Jobs only have the id)

# these jobs are iterable so:
job = [for job in jobs if job.id == id]

job.logs()

And that's it. WDYT?

akihikokuroda commented 2 months ago

Is this proposal adding jobs method to QiskitFunction in addition to the current existing get_jobs method?

Tansito commented 2 months ago

What we were commenting in the issue is that get_jobs should be renamed to jobs for the Catalog (to maintain the coherence with Runtime). But for this conversation we can just assume jobs = function.get_jobs(), they are the same.

Tansito commented 2 months ago

And later we can add a filter there to send the id I assume by now as you were proposing and return an array with one element.

akihikokuroda commented 2 months ago

OK. Let me try this.

Tansito commented 2 months ago

@akihikokuroda correct if I'm wrong, the PR is not solving this issue, right? We should detach this from the PR I think. It solves #1463 and #1479

akihikokuroda commented 2 months ago

I believe this is fixed by the PR. The function.jobs() by the provider returns the all jobs of the function.

Tansito commented 2 months ago

Yes, but this issue is about how to return a specific job. I think that is more related with #1463

akihikokuroda commented 2 months ago

I see. You are right.

akihikokuroda commented 2 months ago

I can create another PR to filter the specific job at the client side later.

Tansito commented 2 months ago

Don't worry Aki. With just the changes that I asked to you in the PR is more than enough by now before your vacations.

Let this issue for when you comeback or another member of the team, there is no need to rush it 😄

akihikokuroda commented 1 month ago

I got confused again :-(

How can I determine 1 if the user is a member of the provider. (I assume the member is allowed to upload or update the function provided by the provider.) 2 if the user has the permission to run or view the functions provided by the provider.

Thanks!

Tansito commented 1 month ago

In this case we are talking about Jobs, not programs so basically the logic here is for this specific end-point /job/:id:

  1. If the author of the job is the user then we can return it (that's what we have right now)
  2. If the user is a provider of function's job it can be retrieved too. And you can check it like: Job > Program > Provider > admin_groups and check if the user is a provider with a logic similar to this: https://github.com/Qiskit/qiskit-serverless/blob/main/gateway/api/serializers.py#L61