Allow api version overrides #1104
Conversation
This optional param allow the default value inherited from Context to be overridden on a per-query basis.
ewalk153
force-pushed
the
ew-add-api-version-argument
branch
from
32a72f9
to
6b6546e
Compare
|
If we align on this approach, I’ll update the release notes. Changelog and docs are now up to date. |
ewalk153
changed the title
ewalk153
force-pushed
the
ew-add-api-version-argument
branch
from
7291714
to
042a69a
Compare
This matches the approach taken the js client and will serve more use cases than on the query.
ewalk153
force-pushed
the
ew-add-api-version-argument
branch
from
042a69a
to
29b0dd4
Compare
| @@ -45,12 +45,21 @@ def test_path_starting_at_admin_overrides_default | |||
| assert_equal(response_headers.to_h { |k, v| [k, [v]] }, response.headers) | |||
| end | |||
|
|
|||
| def test_api_version_can_be_overrriden | |||
| @api_version = "2022-07" | |||
There was a problem hiding this comment.
Using an instance variable was expedient here. We can always rewrite this as an argument to run_test.
There was a problem hiding this comment.
Some nits w.r.t. docs.
Also added some suggestions to log a debug message if the version is being overridden (informational for dev), or to log a debug message if it's being overridden by the same value as in Context (i.e., when it's unnecessary) ... aligns with Node library
|
Hmm, just noticed that the Node client has the debug messaging for |
- add debug logging when the override is a NOOP - improve documentation instructions Co-authored-by: Kevin O'Sullivan <56687600+mkevinosullivan@users.noreply.github.com>
ewalk153
force-pushed
the
ew-add-api-version-argument
branch
from
6ec3c19
to
e8e4c5f
Compare
We should not render log output to $stdout in tests.
ewalk153
force-pushed
the
ew-add-api-version-argument
branch
from
904c2a8
to
e12ad08
Compare
| @@ -152,6 +152,7 @@ def test_send_a_warning_if_log_level_is_invalid | |||
| scope: [], | |||
| is_private: false, | |||
| is_embedded: true, | |||
| logger: ::Logger.new(T.let(StringIO.new, StringIO)), | |||
There was a problem hiding this comment.
Redirect logs to a StringIO to quiet the minitest output. In the future, we can assign the StringIO to an instance variable before passing it in, and check its output to verify logging messages.
Description
Same feature need as Shopify/shopify-api-js#660 in the js client.
In order to develop again unstable features, it's important to be able to make some requests against the unstable version. Furthermore, it's helpful to be able to adopt the latest version of the ApiVersion per client while migrating versions.
This approach adds an optional parameter to the client for both graphql and rest implementations.
Shopify employees need this to layer on experimental features.
The expected outcome of this PR is to allow developers to experiment with new APIs while making most requests against a long term stable api version.
We may introduce an option to override per query in the future.
How has this been tested?
There is an automated test to confirm this has the desired behavior.
Checklist: