goshippo / shippo-python-client

Shipping API Python library (USPS, FedEx, UPS and more)
https://goshippo.com/docs
MIT License
123 stars 70 forks source link

Shipment.create does not return all rates #56

Closed vsoch closed 4 years ago

vsoch commented 4 years ago

This is relevant for shippo==2.0.0rc1

When I create a shipment, as per the examples, I'd expect to get a list of all the rates available to me.

    shipment = shippo.Shipment.create(
               address_from = addresses["From"],
               address_to = addresses["To"],
               parcels = [parcel_default],
               api_key = SHIPPO_TOKEN,
               extra = extra)

This doesn't actually happen - I'm returned just 3, and they are for one provider (usps). Your support suggested that I filter carriers, but this is hugely buggy - if I were to want even more than one carrier I'd likely only get three from whichever comes first.

To get the full list of rates (N=10) I'm forced to take the object_id and pass it back into the Shipment class:

rates = shippo.Shipment.get_rates(shipment.object_id, api_key=SHIPPO_TOKEN)['results']

And then manually update the rates

shipment['rates'] = rates

This is hugely unintuitive, and for someone using your Python client, it's confusing, as they will likely go through weeks to a month of back and forth with you to figure out why the rates aren't appearing. I've provided inordinate details directly with your support team if you need more details. Thanks!

assislucas commented 4 years ago

@vsoch Thanks for raising this issue.

What's the status of the shipment object after you call shippo.Shipment.create?

vsoch commented 4 years ago

heyo @assislucas thanks for jumping in! The status is "QUEUED"

vsoch commented 4 years ago

It looks like there is delay in your API returning the FedEx rates - the original response doesn't include Fedex, we need to query again. Here I am taking the shipment.object_id and getting it again, and this time seeing fedex:

retry = shippo.Shipment.retrieve(api_key=os.environ.get('SHIPPO_TOKEN'), object_id=shipment.object_id)

# here are the providers for the retry request with the same object_id
[x['provider'] for x in retry['rates']]                                                               
['FedEx',
 'FedEx',
 'FedEx',
 'FedEx',
 'FedEx',
 'FedEx',
 'FedEx',
 'USPS',
 'USPS',
 'USPS']

and here were the original providers

[x['provider'] for x in shipment['rates']]                                                            
['USPS', 'USPS', 'USPS']

So I think it would be helpful to document this somewhere, that retrieving a QUEUED status means that the rates aren't fully populated, and the client should wait and then re-request the shipment or the rates. Thoughts? Do you know why this is happening on your side, and the response isn't returned after the rates are generated?

assislucas commented 4 years ago

hey @vsoch :)

Okay that explains it. When you make the shippo.Shipment.create API call it's an asynchronous Shipment creation, so all rates all be available when the status is "SUCCESS". Since it's asynchronous you need to poll the Shipment using the object_id until the status becomes success then all rates will be available.

It's expected that different providers' rates appear at different times because each one has a different response time. Once the shipment status is SUCCESS you should expect that all rates for the valid providers which support this shipment are available.

Let me know if this explanation helps.

vsoch commented 4 years ago

It does! Where is this documented for the user looking to get rates?

assislucas commented 4 years ago

It's available in the Shippo API Docs: https://goshippo.com/docs/reference?version=2018-02-08#shipments

vsoch commented 4 years ago

The average user is going to be following the instructions here to generate a shipment, and there is no indication to check a status and retry to get all rates. Given the stress / trouble this caused my group, I'd suggest adding a note there, something like:

If you don't see rates for all carriers, take the shipment.object_id and request it again, and continue until status goes from QUEUED to SUCCESS to be sure that the rates are done being generated.

We in fact did look over the docs you linked many times, and didn't think to look at status (or that it was relevant at all).

assislucas commented 4 years ago

@vsoch That page doesn't talk about the status because the shipment is created in this way:

shipment = shippo.Shipment.create(
    address_from = address_from,
    address_to = address_to,
    parcels = [parcel],
    async = False
)

Note the async = False parameter. When async = False is passed the shipment is created synchronously, so when you receive the response it will have all available rates. That's an alternative to the polling solution I mentioned previously.

vsoch commented 4 years ago

ah ok, so async must be True by default? The average user won't know that, and might be confused if inputting data into the function and not getting the rates returned.

I understand you are explaining to me why this should be crystal clear, but in reality it wasn't - it led to weeks of support from your staff (with still no answer until I've talked to you) and much extra debugging / work to try and understand why. If you could add one or two sentences to explain how this works, in plain english, this would hugely help future users (and would have saved us much debugging time).

Are the docs open source? If you don't have the bandwidth to make the change I can do it for you.

assislucas commented 4 years ago

@vsoch Unfortunately those docs are not open source, I'll follow up internally to update them.

I understand your frustration and appreciate the feedback, we will add a note about the sync vs async API calls.

vsoch commented 4 years ago

Thank you! I really appreciate it, and thank you again for addressing the issue. I think we’ve addressed both the original question and a future update to the docs to help others avoid it, so I’m going to close the issue.