Add multi region support #483

randomir · 2021-08-28T01:11:17Z

After we implement Simplify 'config create' step #304, we'll add region prompt in the "advanced" dwave config create flow. This is not blocking the review, or even merge.

codecov-commenter · 2021-08-28T01:15:43Z

Codecov Report

Merging #483 (6019484) into master (d8b15e9) will increase coverage by 0.64%.
The diff coverage is 93.28%.

@@            Coverage Diff             @@
##           master     #483      +/-   ##
==========================================
+ Coverage   85.51%   86.15%   +0.64%     
==========================================
  Files          24       24              
  Lines        3099     3244     +145     
==========================================
+ Hits         2650     2795     +145     
  Misses        449      449

Impacted Files	Coverage Δ
dwave/cloud/api/__init__.py	`100.00% <ø> (ø)`
dwave/cloud/api/exceptions.py	`100.00% <ø> (ø)`
dwave/cloud/api/client.py	`91.60% <84.61%> (+2.01%)`	⬆️
dwave/cloud/client/base.py	`90.23% <88.70%> (-0.01%)`	⬇️
dwave/cloud/api/resources.py	`92.96% <93.54%> (+1.37%)`	⬆️
dwave/cloud/api/constants.py	`100.00% <100.00%> (ø)`
dwave/cloud/api/models.py	`97.67% <100.00%> (+0.11%)`	⬆️
dwave/cloud/cli.py	`60.91% <100.00%> (+0.24%)`	⬆️
dwave/cloud/config.py	`97.61% <100.00%> (+0.32%)`	⬆️
dwave/cloud/exceptions.py	`100.00% <100.00%> (ø)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8b15e9...6019484. Read the comment docs.

mdecandia · 2021-09-29T18:49:28Z

It appears that specifying a region when instantiating the Client from config does not override the endpoint specified in dwave.conf. For example, if dwave.conf specifies 'https://eu-central-1... but the client is instantiated as Client.from_config(region="na-west-1"), the client will use the EU endpoint.

randomir · 2021-10-01T11:05:07Z

dwave/cloud/client/base.py

+
+        return data
+
+    def get_regions(self, refresh=False):


TODO: make static after we refactor config (decouple from client construction).

In particular, we add: - Client.get_regions() - region config option and DWAVE_API_REGION env var - metadata_api_endpoint config option

silviacorbellaro

DiskCache is approved from IP.

dwave/cloud/client/base.py

For example `region` in env should override `endpoint` in config file. However `endpoint` in file will override `region` in file.

randomir · 2021-10-14T00:09:34Z

@mdecandia, support for mutually exclusive config options (exclusive on different levels) should now be in.

So, for example, region in env will override endpoint in file. But endpoint in file will override region in file.

JoelPasvolsky

Just a few things that popped out at me while I was going through these updates.

dwave/cloud/api/client.py

dwave/cloud/api/resources.py

dwave/cloud/client/base.py

releasenotes/notes/add-multi-region-support-8c49dbd7bbcfc34b.yaml

Exposed on `DWaveAPIClient.session.history` as well.

mdecandia

Looks good and works well!

JoelPasvolsky · 2021-10-18T18:19:22Z

dwave/cloud/api/constants.py

-# Default SAPI endpoint
-DEFAULT_API_ENDPOINT = 'https://cloud.dwavesys.com/sapi/'
+# Default API endpoints
+DEFAULT_SOLVER_API_ENDPOINT = 'https://cloud.dwavesys.com/sapi/'


Should this be updated to https://na-west-1.cloud.dwavesys.com/sapi/v2?

Both work. We can update the default once MR is GA.

we will be deprecating the old one at some point - charles is arranging a meeting to discuss timing

randomir force-pushed the add-multi-region-support branch from d4ac771 to 0fb81c4 Compare August 30, 2021 22:20

randomir requested review from arcondello and mdecandia September 21, 2021 23:57

randomir commented Oct 1, 2021

View reviewed changes

randomir requested a review from silviacorbellaro October 12, 2021 19:50

randomir added 13 commits October 12, 2021 13:05

Generalize API client to handle multiple D-Wave APIs

567199d

Add metadata api client (/regions resource and model)

af92a7f

Add mock and live tests for Regions resource

894c48e

Add region support to the Client

3bcffa9

In particular, we add: - Client.get_regions() - region config option and DWAVE_API_REGION env var - metadata_api_endpoint config option

Update docs for multi-region

81d06ae

Add ctx mgr interface to resource base so session can be closed

a9e7ebb

Add config.get_cache_dir()

39886d3

Add @cached.ondisk, a long-term cache backed by sqlite3

01712dc

Cache region metadata on disk

9d314b9

Re-cast region metadata api access errors

4a3f2ba

Add/update region/endpoint selection tests

efd00b8

Add end-to-end region selection tests

943edb0

Fallback to default sapi endpoint if metadata api is down

1f3e318

silviacorbellaro approved these changes Oct 12, 2021

View reviewed changes

randomir force-pushed the add-multi-region-support branch from 370980a to 1f3e318 Compare October 13, 2021 16:10

JoelPasvolsky reviewed Oct 13, 2021

View reviewed changes

dwave/cloud/client/base.py Show resolved Hide resolved

Improve generic dwave api client exception logging

433b562

JoelPasvolsky reviewed Oct 13, 2021

View reviewed changes

dwave/cloud/client/base.py Outdated Show resolved Hide resolved

randomir added 4 commits October 13, 2021 16:28

Make isolated_environ both context manager and decorator

30f1247

Add support for mutually exclusive config options

9663cab

For example `region` in env should override `endpoint` in config file. However `endpoint` in file will override `region` in file.

Simplify client by deferring config merge to load_/update_config utils

6012ce0

Test mutually exclusive config options

cd26e65

Fix RequestTimeout in LoggingSession to include request

81cfb02

JoelPasvolsky reviewed Oct 14, 2021

View reviewed changes

randomir added 6 commits October 14, 2021 12:49

Keep request history in LoggingSession (up to history_size)

1c5491b

Exposed on `DWaveAPIClient.session.history` as well.

Log details for failed request, even without response

1828a6d

Test session.history tracking

c352c35

Use py36-compatible version of namedtuple

07c3156

Update multi-region docs per review comments

96f74a3

Add env var to control metadata_api_endpoint

8f8da94

mdecandia approved these changes Oct 16, 2021

View reviewed changes

randomir force-pushed the add-multi-region-support branch from 6087dfd to 893b46e Compare October 18, 2021 17:23

randomir added 3 commits October 18, 2021 11:10

Use click.prompt for CLI user input

33b3d1c

Remove unneeded text input util (replaced with click)

1efcd65

Prompt for 'region' in 'config create --full' flow

4e868f7

randomir force-pushed the add-multi-region-support branch from 893b46e to 4e868f7 Compare October 18, 2021 18:10

JoelPasvolsky reviewed Oct 18, 2021

View reviewed changes

Propagate headers from config to Metadata API for regions fetch

6019484

randomir merged commit 4dee3ce into dwavesystems:master Oct 18, 2021

randomir deleted the add-multi-region-support branch October 18, 2021 18:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi region support #483

Add multi region support #483

randomir commented Aug 28, 2021 •

edited

codecov-commenter commented Aug 28, 2021 •

edited

mdecandia commented Sep 29, 2021 •

edited

randomir Oct 1, 2021 •

edited

silviacorbellaro left a comment

randomir commented Oct 14, 2021

JoelPasvolsky left a comment

mdecandia left a comment

JoelPasvolsky Oct 18, 2021

randomir Oct 18, 2021

fionahanington Oct 18, 2021

Add multi region support #483

Add multi region support #483

Conversation

randomir commented Aug 28, 2021 • edited

codecov-commenter commented Aug 28, 2021 • edited

Codecov Report

mdecandia commented Sep 29, 2021 • edited

randomir Oct 1, 2021 • edited

Choose a reason for hiding this comment

silviacorbellaro left a comment

Choose a reason for hiding this comment

randomir commented Oct 14, 2021

JoelPasvolsky left a comment

Choose a reason for hiding this comment

mdecandia left a comment

Choose a reason for hiding this comment

JoelPasvolsky Oct 18, 2021

Choose a reason for hiding this comment

randomir Oct 18, 2021

Choose a reason for hiding this comment

fionahanington Oct 18, 2021

Choose a reason for hiding this comment

randomir commented Aug 28, 2021 •

edited

codecov-commenter commented Aug 28, 2021 •

edited

mdecandia commented Sep 29, 2021 •

edited

randomir Oct 1, 2021 •

edited