Skip to content

API version 2.1.0 See https://docs.patch.io/#/changelog for changes #72

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 4, 2023

Conversation

venalen
Copy link
Contributor

@venalen venalen commented Mar 24, 2023

What

API version 2.1.0. See https://docs.patch.io/#/changelog for changes

Why

Version 2.1.0 adds support for

  • vintage_start_year and vintage_end_year for orders
  • vintage_start_year and vintage_end_year for order line items
  • vintage_start_year and vintage_end_year for inventory
  • carrier_scac field to patch.estimates.create_road_shipping_estimate

SDK Release Checklist

  • Have you added an integration test for the changes?
  • Have you built the library locally and made queries against it successfully?
  • Did you update the changelog?
  • Did you bump the package version?
  • If endpoints were removed, did you manually remove the corresponding files? (this should be rare)
  • For breaking changes, did you plan for the release of the new SDK versions and deploy the API to production?

Sorry, something went wrong.

@venalen venalen force-pushed the venalen/python-sdk-2.1.0 branch from f7cbdf2 to 7384e35 Compare March 24, 2023 20:10
@@ -23,7 +23,7 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: '3.6'
python-version: '3.10'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appears to be a long-running problem with versions 3.6 and 3.9 as seen in this thread. In our case I don't believe it hurts to bump to 3.10

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. But I'm not sure we should change the required version for the package: https://github.com/patch-technology/client-code-generation/pull/70#pullrequestreview-1359412217

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, that makes sense. I'll also poke around to make sure that we need this at all; the publish.yml file still has a version of 3.6, so I'm wondering if this needs any change at all

@venalen venalen force-pushed the venalen/python-sdk-2.1.0 branch 5 times, most recently from e9f0839 to 91cc346 Compare March 25, 2023 02:16
@venalen venalen force-pushed the venalen/python-sdk-2.1.0 branch from 91cc346 to f874f16 Compare March 25, 2023 20:01
@venalen venalen marked this pull request as ready for review March 27, 2023 15:43
@venalen venalen requested a review from a team March 27, 2023 15:44
Copy link
Contributor

@pcothenet pcothenet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion on python version before merging.

@venalen venalen force-pushed the venalen/python-sdk-2.1.0 branch 5 times, most recently from 7e12496 to 56a943e Compare March 27, 2023 18:09
@venalen venalen force-pushed the venalen/python-sdk-2.1.0 branch from 56a943e to c3a9801 Compare March 27, 2023 18:11
@venalen
Copy link
Contributor Author

venalen commented Mar 27, 2023

@pcothenet thanks for the nudge to look into the python issues more deeply! It looks like pinning the ubuntu version remedies the situation and avoids an unnecessary python upgrade. It seems like there's an underlying issue with the python tarballs for ubuntu-latest which will hopefully be addressed in the near future, but for now we can use another os version and it should have impact on the SDK/python itself. WDYT?

@pcothenet
Copy link
Contributor

@pcothenet thanks for the nudge to look into the python issues more deeply! It looks like pinning the ubuntu version remedies the situation and avoids an unnecessary python upgrade. It seems like there's an underlying issue with the python tarballs for ubuntu-latest which will hopefully be addressed in the near future, but for now we can use another os version and it should have impact on the SDK/python itself. WDYT?

Sounds good to me.

I think you could also keep the test running on 3.10, but not force that as a min version for the SDK (ultimately, we should have a test matrix of multiple Python versions for this SDK)

@venalen
Copy link
Contributor Author

venalen commented Mar 27, 2023

Ah yeah a test matrix would be ideal and hopefully is in our near future. For now I was thinking that it would be best to have the tests run on the same version that the package is published to maintain consistency!

@venalen venalen force-pushed the venalen/python-sdk-2.1.0 branch from e4de7fe to 5d4f6c9 Compare April 4, 2023 21:08
@@ -89,7 +89,7 @@ def test_create_and_retrieve_shipping_estimate(self):
create_order=False,
)
self.assertEqual(estimate.data.order, None)
self.assertEqual(estimate.data.type, "shipping")
self.assertEqual(estimate.data.type, "ecommerce")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're now re-routing to the ecommerce endpoint: https://github.com/patch-technology/patch/pull/5588

@venalen venalen force-pushed the venalen/python-sdk-2.1.0 branch from 5d4f6c9 to 8b84a2a Compare April 4, 2023 21:14
@venalen venalen requested a review from a team April 4, 2023 21:18
@venalen venalen force-pushed the venalen/python-sdk-2.1.0 branch from 8b84a2a to a9ebe61 Compare April 4, 2023 21:53
@venalen venalen merged commit cf49ec7 into main Apr 4, 2023
@venalen venalen deleted the venalen/python-sdk-2.1.0 branch April 4, 2023 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants