CQCL / pytket-iqm

pytket-iqm, extensions for pytket quantum SDK
Apache License 2.0
2 stars 4 forks source link

Update iqm-client to 9.1 and expand Readme #28

Closed freetonik closed 1 year ago

freetonik commented 1 year ago

Hello,

IQM's demo server has been upgraded a couple of days ago. It requires a newer version of IQM Client. The most important change is that it's no longer necessary for the client to provide the settings file.

This PR upgrades pytket-iqm to the latest iqm-client 9.1. In addition to the removal of the settings file, the PR also contains the following:

Remote tests against IQM demo env successfully pass in this branch.


By the way, user authentication can now be handled externally withcortex-cli. This requires making some changes to IQMConfig in pytket-iqm, which I decided not to do in this PR. Ideally, pytket-iqm would support both ways (like cirq-on-iqm or qiskit-on-iqm do):

  1. Current way of providing auth url, username and password which are passed down to iqm-client.
  2. Not providing auth url, username and password, so that iqm-client falls back to using the IQM_TOKENS_FILE env variable for authentication.

If you have any questions, I'd be happy to answer them.

Thanks!

cqc-melf commented 1 year ago

Happy to approve the run of the checks after we key in the CI is set to the value of the old secret as suggested.

cqc-melf commented 1 year ago

Can you take care of the failing tests? I can only work on your branch with suggestions. If there is anything I can help you with, please let me know!

freetonik commented 1 year ago

I've fixed the lint errors, but remote tests fail with what seems like a auth problem. Can you confirm that PYTKET_REMOTE_IQM_AUTH_SERVER_URL secret's value is https://auth.demo.qc.iqm.fi?

cqc-melf commented 1 year ago

I can't read the values from the secrets, but I have set the value now to https://auth.demo.qc.iqm.fi and started a rerun.

freetonik commented 1 year ago

That didn't help :(

Not sure what the problem is right now. If URL were unreachable, or credentials were incorrect, then a different error would've occurred. The absence of ClientAuthenticationError means the authentication went through. But then the HTTP response to any request is a HTML page with the login form...

freetonik commented 1 year ago

I ran the the complete .github/workflows/build-test nomypy locally, and it passed. So I think the problem is in the CI secrets somewhere.

cqc-melf commented 1 year ago

Are we sure that the https:// should be in the url?

cqc-melf commented 1 year ago

If you want to, you can send me the credential via email and I update them again. I cant read what is stored in there at the moment.

cqc-melf commented 1 year ago

I am not sure if this might be a GitHub issue, that secrets are not allowed to be used in external PRs.

Could you maybe open a second PR into the branch update/iqm-client which I just created? I could directly merge that and run the checks on that branch to find out of there are any GitHub restriction making our life more complicated then we would like to at the moment.

cqc-melf commented 1 year ago

I was able to create the PR myself, will run the checks now.

cqc-melf commented 1 year ago

See: https://github.com/CQCL/pytket-iqm/pull/30

freetonik commented 1 year ago

Great! Should I close the current PR without merging, then?

cqc-melf commented 1 year ago

Are there any unsolved questions in this PR?

cqc-melf commented 1 year ago

From my side this one can be closed. I need one other person to approve the other PR, and we could do a release after that. if there are people waiting for these changes.

freetonik commented 1 year ago

Alright then, thank you! I'll keep my eye on the merge and release, hope it happens soon.