dwavesystems / dwave-cloud-client

A minimal implementation of the REST interface used to communicate with D-Wave Solver API (SAPI) servers.
https://docs.ocean.dwavesys.com/projects/cloud-client/en/stable/
Apache License 2.0
59 stars 40 forks source link

Add LeapAPI support #540

Closed randomir closed 1 year ago

randomir commented 1 year ago

In this PR we add LeapAPIClient base client, LeapAccount resource class, and the appropriate Leap API models (types) in order to support:

codecov-commenter commented 1 year ago

Codecov Report

Merging #540 (3d4d545) into master (d1bf2ee) will increase coverage by 0.16%. The diff coverage is 96.61%.

@@            Coverage Diff             @@
##           master     #540      +/-   ##
==========================================
+ Coverage   86.69%   86.86%   +0.16%     
==========================================
  Files          24       24              
  Lines        3473     3524      +51     
==========================================
+ Hits         3011     3061      +50     
- Misses        462      463       +1     
Impacted Files Coverage Δ
dwave/cloud/api/client.py 94.05% <93.75%> (+0.34%) :arrow_up:
dwave/cloud/api/resources.py 95.72% <95.83%> (-0.01%) :arrow_down:
dwave/cloud/api/__init__.py 100.00% <100.00%> (ø)
dwave/cloud/api/constants.py 100.00% <100.00%> (ø)
dwave/cloud/api/models.py 98.11% <100.00%> (+0.33%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

charleswhchan commented 1 year ago

I believe Bulat might have some input for this feature. Can you add him to the PR as well?

charleswhchan commented 1 year ago

We shouldn't release this until zero downtime for Leap is completed (Q3 '23). Otherwise, calls to /leap will end up failing during by weekly deployment. Bad UX.

randomir commented 1 year ago

We shouldn't release this until zero downtime for Leap is completed (Q3 '23). Otherwise, calls to /leap will end up failing during by weekly deployment. Bad UX.

@charleswhchan, Aren't SAPI calls failing during SAPI deploys?!

charleswhchan commented 1 year ago

sapi-ws has zero downtime implemented and scheduler has a very short delay before restart, so all /sapi calls should be handled and processed as usual.

where as calls to /leap will show the maintenance page and probably return a 5xx error

randomir commented 1 year ago

We already use Leap API from the IDE, so I think it's fine to release this before the end of Q3. It's not like it will be visible and used by default. Currently it's not even documented; intended practically for internal use.

arcondello commented 1 year ago

We shouldn't release this until zero downtime for Leap is completed (Q3 '23). Otherwise, calls to /leap will end up failing during by weekly deployment. Bad UX.

My 2c is that a user who is using this feature (especially given its current semi-private nature) would understand that when Leap is down they would get an error message. So IMO while it's not an ideal user experience, the relative benefits are well worth it.

charleswhchan commented 1 year ago

My 2c is that a user who is using this feature (especially given its current semi-private nature) would understand that when Leap is down they would get an error message. So IMO while it's not an ideal user experience, the relative benefits are well worth it.

Rad, Bulat and myself will be meeting on Monday -- we can reach a decision then.