Skip to content

Conversation

@jkelleyj
Copy link
Contributor

@jkelleyj jkelleyj commented Aug 1, 2016

When adding labels to a manifest, we can receive partially successful responses from DHL. These changes add labels to the manifest and only appends the shipping labels that were successfully attached to the closeout.
http://api.dhlglobalmail.com/docs/closeout.html

I'm open to alternate design approaches here, but I needed a way to differentiate between what labels were actually appended to the manifest and which failed to attach (normally because they were closed out previously).

An alternate approach I abandoned because it seemed too obscured is to take an optional block as a parameter that is passed failed labels so the caller can decide what to do with them.

The main downside to the way I have attached labels to the returned manifest object is that this data is not actually returned by the API. Rather, it is constructed from the input to .create(labels). This could lead to inconsistency in what data is present since a caller could pass any ducktyped object that has the impb construct on it to be added to the manifest. I'm not too worried about this since the caller both injects that information and then uses it after the call, but I'm open to ways to clean it up.

Also includes a small refactor on the manifest.rb to chunk out the individual steps in the three step manifest process for clarity.

@jkelleyj
Copy link
Contributor Author

jkelleyj commented Aug 1, 2016

I also considered handling this through this library's error handling processes, but DHL's api returns a 200 status code during partial failures that this is used to handle. It also seemed problematic to raise an exception when some were successfully added and their status was a success.

@fdeschenes
Copy link
Member

That's an interesting issue. I'll try to think of a solution and see what I can come up with. What's a typical reason why a label would fail to attach?

@jkelleyj
Copy link
Contributor Author

jkelleyj commented Aug 5, 2016

Usually it was attached previously to a different manifest. This is most
likely when a system uses the close all and the other system uses the three
step process. Alternatively it can occur when some sort of failure
occurred during the first manifest attempt and the labels are now stuck on
that close out. I know it is far from the happy path but seems worth
handling.

I'll be submitting a PR soon that performs the Close All action since we
are likely moving in that direction.

On Friday, August 5, 2016, Francois Deschenes notifications@github.com
wrote:

That's an interesting issue. I'll try to think of a solution and see what
I can come up with. What's a typical reason why a label would fail to
attach?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#3 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAjW5JkiPblDNqYTThgGgWPZWODeioc9ks5qc3xOgaJpZM4JZ7rM
.

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.

2 participants