Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Lots of API specification updates #13

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Lots of API specification updates #13

wants to merge 23 commits into from

Conversation

kendallb
Copy link
Contributor

@kendallb kendallb commented Jul 8, 2021

These are all the changed we have had to make to the specification to get it working properly in the real world. It has been a while since I created a PR, and I just rebased onto the official code to pick up all the changes for IOSS updates. But all the changes I made are probably relevant to anyone else using the YAML spec, since the stuff I changed is wrong (and certainly difficult to use with C#).

@kendallb
Copy link
Contributor Author

kendallb commented Jul 8, 2021

BTW, I noticed the API spec added a bunch of new enumeration values, but they were added in the MIDDLE of the existing enumerations. This is a really bad idea for code generation in C# because it becomes a numeric enumeration when it is parsed, so while the values passed over the wire are strings, the internal values are numeric. So if you add a new value in the middle, the numeric values change. This means anyone consuming the C# API library will need to recompile to use the updated API definitions when the enumerations change. To fix this, any changes enumerations really need to have new values added to the end?

@kendallb kendallb force-pushed the feature/kendallb/api-specification-updates branch from e01f383 to 4d264b6 Compare July 8, 2021 02:10
@kendallb
Copy link
Contributor Author

I am going to rebase on top of the latest changes, but is there a reason my changes cannot be accepted upstream? All of these changes are necessary changes to correctly build a C# client library that works in the wild.

@kendallb kendallb force-pushed the feature/kendallb/api-specification-updates branch 2 times, most recently from 4c53e90 to fc87313 Compare March 27, 2024 01:34
kendallb added 4 commits March 26, 2024 21:59
@kendallb
Fixed country_code, site_id and mailer_id to be common type, so it wo…
9f3498c 
…rks with C# client generation
@kendallb
This item is nullable as the actual code can return null here (includ…
ba0fc45 
…ed in JSON, but null). So either we need to mark it as nullable here or the service should not be including the value.

(cherry picked from commit aabd5e7)
@kendallb
These all need to be nullable also as the service responses actually …
c867514 
…have nulls in them

(cherry picked from commit 0f7b9bb)
@kendallb
These can be null for UPS SurePost, so the spec needs to handle that
1bd40d7 
(cherry picked from commit 9ca0d97)
@kendallb kendallb force-pushed the feature/kendallb/api-specification-updates branch from fc87313 to 270b6d6 Compare March 27, 2024 02:00
kendallb added 18 commits March 26, 2024 22:04
@kendallb
The customs_item_id is marked as required and not-nullable, but it re…
841579d 
…ally should be nullable so it can be left out of the request.
@kendallb
There is already a type for carrier code so it should be used and NOT…
04ebd2a 
… a string!

(cherry picked from commit 8ea157a)
(cherry picked from commit c7af338)
@kendallb
Add support for undocumented test_labels parameters
1e5d0d7 
(cherry picked from commit 6297be8)
(cherry picked from commit f85ed1a)
(cherry picked from commit 603d5ff)
@kendallb
Warehouse ID is *not* required for manifesting, and this will fail to…
3d7668f 
… parse the responses sent back if you manifest for specific labels.

(cherry picked from commit 20dcf31)
(cherry picked from commit 26b6dab)
(cherry picked from commit 85576dc)
@kendallb
Tracking numbers need to be optional to correctly parse the response …
bb9f92f 
…when a label is a test label with USPS.

(cherry picked from commit 444f133)
(cherry picked from commit 64a2707)
(cherry picked from commit 3e60dbb)
@kendallb
These should not be marked as obsolete as it will cause the compiler …
0fce42c 
…to throw and error if you use it. And it still does work.

(cherry picked from commit 433dc5e)
(cherry picked from commit a5405f1)
(cherry picked from commit c66e6b6)
@kendallb
Fixed incorrectly specified 204 responses as they are actually empty …
ea928b7 
…and there is no response content to parse

(cherry picked from commit 2147f27)
(cherry picked from commit 47f4a77)
(cherry picked from commit 3c59f10)
@kendallb
This must be nullable, as it comes back null for USPS manifest docume…
a84e0fd 
…nts from Endicia.

(cherry picked from commit aad88e3)
(cherry picked from commit 3a8eefa)
(cherry picked from commit 4fbc871)
@kendallb
Gah, this *also* needs to be nullable
a72118d 
(cherry picked from commit ac295be)
(cherry picked from commit fb28b95)
(cherry picked from commit 83d9831)
@kendallb
Some countries do not have states, so this can fail if null is passed…
2250934 
… for an empty country

(cherry picked from commit a4a1e51)
(cherry picked from commit bf993ba)
(cherry picked from commit 5ff9e58)
@kendallb
Added in the missing carrier information to the error responses
fd15e92 
(cherry picked from commit c2628b1)
(cherry picked from commit b9f6822)
(cherry picked from commit 126abfe)
@kendallb
Add support for missing label_image_id which is not documented on the…
3ea520e 
… web site.

(cherry picked from commit 83d528a)
(cherry picked from commit fbfc6b9)
(cherry picked from commit 244c52c)
@kendallb
These are NULLABLE, as they can come back as NULL from the addressd v…
fd2d4af 
…alidation service. I am not sure why you guys changed then to not allow null???

(cherry picked from commit 300055b)
(cherry picked from commit ecb75a1)
(cherry picked from commit a44db2b)
@kendallb
This must also be nullable
ac4c79e 
(cherry picked from commit ebc95fa)
(cherry picked from commit 8e68602)
(cherry picked from commit 8ddb356)
@kendallb
Add in missing error response
5a55b40 
(cherry picked from commit 18741f4)
(cherry picked from commit 7a87490)
(cherry picked from commit 8f414a1)
@kendallb
More fields need to be nullable
2e771c3 
(cherry picked from commit 27dd7f0)
(cherry picked from commit cd6ed10)
(cherry picked from commit 202bf0e)
@kendallb
Yet more fields need to be nullable
aabb556 
(cherry picked from commit 0847e6e)
(cherry picked from commit 76d3238)
(cherry picked from commit 485791a)
@kendallb
Yet more nullable fields in the wild
c331d61 
(cherry picked from commit 4c53e90)
(cherry picked from commit fc87313)
(cherry picked from commit 270b6d6)
@kendallb kendallb force-pushed the feature/kendallb/api-specification-updates branch from 270b6d6 to c331d61 Compare March 27, 2024 02:05
@kendallb
Fix up broken image_content_type and revert the shipping_address_to s…
0a08ba2 
…tuff to maintain binary compatibility for now. Might put that back later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@kendallb