Aggregate Approvals in Malawi

28 views
Skip to first unread message

benle...@gmail.com

unread,
Apr 20, 2017, 9:16:31 PM4/20/17
to OpenLMIS Dev
Hi everyone,

The Malawi team is planning to begin backend work related to https://openlmis.atlassian.net/browse/MW-57 soon. The few implementation details mentioned in the epic may be off – we didn’t realize that the core product’s support for extension modules allows for the ad hoc addition of new endpoints. The planned functionality described by the epic is accurate, though.

My question for the group is whether this is functionality the core project would like to incorporate and maintain. Assuming it were implemented to the standard expected by the core project, would a pull-request for this functionality be approved even though it covers only a small subset of the potential features related to aggregated approvals? Part of the discussion in the "New Feature Request from Malawi" section here mentions that “there are additional core requirements that need definition and need to be addressed for this feature set to be acceptable.” On the other hand, it sounds as though a definitive decision wasn’t reached.

I was encouraged to post here, and hope it’s the right venue. I’d be glad to instead post in the Product Committee forum if it’s more appropriate.

Thank you,
Ben

Josh Zamor

unread,
Apr 20, 2017, 10:03:57 PM4/20/17
to benle...@gmail.com, OpenLMIS Dev
Hi Ben,

When I encouraged this, the thought was that the endpoints and functionality that this new Service might provide:


Might actually be appropriate, more expedient and more performant if built directly into the Requisition Service.  Put another way, maybe the new endpoints that the Malawi team were going to put into the mw-openlmis-requisition service, were so un-controvisial and so simple to support within the Requisition service, that the tech committee would be in wild agreement to add them and the Requisition Service team members would be glad to see a PR for them.

To be in wild agreement, we first need a rough idea of what this new endpoint(s) would be.  mw-openlmis-requisition doesn’t have anything yet, so if the Malawi team could outline, in simple text in this forum:

1. New endpoints.  That is the new RESTful Resources or new representations.
2. Draft outline of what the JSON schema would be.

This forum could quickly weigh in if the path would lead to wild agreement before any code was written.

And as you mentioned, even if the Requisition Service didn’t desire the PR, architecturally and performance wise it could make quite a bit of sense to use an Extension module.  Lets cross that bridge if we need to.

Best,
Josh


--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/f56f359f-bc1d-4cea-a39c-572f86959734%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

benle...@gmail.com

unread,
Apr 20, 2017, 11:12:59 PM4/20/17
to OpenLMIS Dev, benle...@gmail.com
Hi Josh,

Thank you very much for your quick response. Your thoughts and questions about our needs for extending the API make a lot of sense. Although we haven't yet determined exactly what the new endpoints will look like, we'll follow up as soon as we have.

Thanks again!

~ Ben

benle...@gmail.com

unread,
Apr 27, 2017, 5:02:14 PM4/27/17
to OpenLMIS Dev, benle...@gmail.com
Hi Josh,

Thanks yet again for your guidance with this. We propose a /requisitions/approve and /requisitions/save endpoint, each of which Sebastian has documented here:


Please don't hesitate to let me know if there's anything else we can do to help gauge the core-team's interest.

Kind regards,
Ben 

Chongsun Ahn

unread,
May 1, 2017, 1:49:38 PM5/1/17
to benle...@gmail.com, OpenLMIS Dev
Hey Ben,

These new API endpoints with their JSON schemas look fine to me; I expect the core team would incorporate them into the Requisition Service.

Shalom,
Chongsun
​
-- ​
There are 10 kinds of people in this world; those who understand binary, and those who don’t.

Software Development Engineer
 
VillageReach Starting at the Last Mile
2900 Eastlake Ave. E, Suite 230,  Seattle, WA 98102, USA
DIRECT: 1.206.512.1536   CELL: 1.206.910.0973   FAX: 1.206.860.6972
SKYPE: chongsun.ahn.vr
Connect on Facebook, Twitter and our Blog

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.

benle...@gmail.com

unread,
May 1, 2017, 2:20:30 PM5/1/17
to OpenLMIS Dev, benle...@gmail.com
Hi Chongsun,

That's great to hear. We're eager to continue contributing to the core project, and I'm glad that this can be another opportunity. Thank you very much for letting me know!

Kind regards,
Ben

Sebastian Brudziński

unread,
May 9, 2017, 11:57:29 AM5/9/17
to openlm...@googlegroups.com

Hello everyone,

We have started the work on those batch endpoints and will be providing a pull request to the core OpenLMIS soon. We have also briefly discussed the new endpoints at the technical comittee call today. We have put some thought into the proper "partial success" HTTP code that we want to use. Since the endpoints we are considering are operating on several instances, the actions on some of them may succeed, while for some other they may fail. This will be reflected in the response body by having two arrays - one containing successfully processed requisitions and another containing errors connected with specific requisitions. Other than the response body though, what should be the HTTP code? There seemed to be 3 main ideas:
 - Always 200 - No matter if all requisition actions succeeded or only some of them, the response would be 200. The errors (if any) would be found in the response body.
 - 400 if at least one failed, 200 if all succeeded - if there's at least one entry in the errors array, the response code would be 400. If all succeeded, it would be 200.
 - 207 (multi-status) which is not an official HTTP response code, but it's in the WebDAV extension to the HTTP protocol.

We are currently leaning towards the first option - always returning 200. Returning 400 on partial success may suggest that the whole request failed, which would not be the case. We also don't think that using 207 would really benefit us much - we would still need to go to the response body to figure out what happened with the request.

Does anyone else have a strong opinion on this or want to propose another approach? We want to follow the same standard for any other batch operations that may appear in OpenLMIS, therefore this decision will impact all future batch endpoints as well and will be documented in the code style guide.

Best regards,
Sebastian.


For more options, visit https://groups.google.com/d/optout.

--

Sebastian Brudziński
Software Developer / Team Leader
sbrud...@soldevelo.com

SolDevelo Sp. z o. o. [LLC]
Office: +48 58 782 45 40 / Fax: +48 58 782 45 41 Al. Zwycięstwa 96/98 81-451, Gdynia
http://www.soldevelo.com

Place of registration: Regional Court for the City of Gdansk KRS: 0000332728, TAX ID: PL5862240331, REGON: 220828585, Share capital: 60,000.00 PLN

Jake Watson

unread,
May 9, 2017, 1:46:09 PM5/9/17
to Sebastian Brudziński, openlm...@googlegroups.com
Would it not be easier to program to option #2?
If I am writing a client, I want to know if there are errors, so any non-200 response would be easier for me to know if there was an error vs having to read each (200) message, then write some additional logic. Or will you have numbers in the message?

Office: +48 58 782 45 40/ Fax: +48 58 782 45 41Al. Zwycięstwa 96/9881-451, Gdynia
http://www.soldevelo.com

Place of registration: Regional Court for the City of GdanskKRS: 0000332728, TAX ID: PL5862240331, REGON: 220828585,Share capital: 60,000.00 PLN

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
Soldevelo_logo_EPS_CMYK.png

Paweł Albecki

unread,
May 9, 2017, 2:44:43 PM5/9/17
to OpenLMIS Dev
Similar dilemma me and Chongsun had while working at this ticket. The issue then was: what should be the http response if user is created but email was not send to that user? For that time we ended up with: 200 and log warn if email was not sent. I'm wondering if introduce 207 status code to openlmis, during work at batch endpoints, wouldn't be helpful also to some other cases like that I mentioned.
Hey Ben,

On Apr 27, 2017, at 2:02 PM, ...@gmail.com wrote:

Hi Josh,

Thanks yet again for your guidance with this. We propose a /requisitions/approve and /requisitions/save endpoint, each of which Sebastian has documented here:


Please don't hesitate to let me know if there's anything else we can do to help gauge the core-team's interest.

Kind regards,
Ben 

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.
To post to this group, send email to ...@googlegroups.com.
--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CB44C260-9E87-45BF-B6F6-C2A14894BD50%40villagereach.org.
For more options, visit https://groups.google.com/d/optout.

Josh Zamor

unread,
May 9, 2017, 4:14:32 PM5/9/17
to Paweł Albecki, OpenLMIS Dev
The more I think about this, the more I like 400.

Reasoning:

  • We generally have full control over what the server sends, and what the client does with the response.  That makes it easy for us to adopt something like 207 and attach our meaning to it, however having full control over producer/consumer doesn’t really lend itself to good adoption reasoning if we want to fully leverage the standard.  e.g.  if some other client used our RESTful HTTP endpoints, and got a 207, there’s nothing they could inherently know about that 207 that would fit with what we meant by it - other than “it’s a mixed bag bub".  We’d just be expanding the litany of statuses our clients would need to understand, needlessly (why comes next).
  • 400 lets the client know explicitly that the operation shouldn’t be tried again, without the client changing the request.  The semantics of this will be especially important when handling the "non-idempotent" POST /requisitions/approve in the example.  This request, if tried again without alteration, could move some of the requisitions which were approved successfully in the first call, to be approved again (and move supervisory node again) in a second followup request that was un-altered.  Or if the supervision structure didn’t have more supervisory nodes or the user didn’t have the approval right on both nodes, it might result in new errors in the followup request that the first didn’t have.  Weather the request is POST or PUT, we don’t want clients blindly re-trying, and 400 means that every-time.
  • 200 doesn’t have this explicit statement about re-trying.  Case closed.  This bullet point is here because bullet points are nicer in multiples of threes...

So my vote is more firmly on the 400 side.

As a side note, I wonder about using the POST in /requisitions/approve.  While returning a 400 for this operation is explicit to a client that it shouldn’t be tried again un-altered, what makes it explicit to the client that it shouldn’t be re-tried if it times out?  Re-trying as the client without an explicit status code could lead to exactly the same situation(s) I ruminated on above.  It could become a PUT operation - though I think that would likely increase the verbosity of the representation.  An alternative that might be good-enough to that extra work could be to be very explicit in the documentation (RAML) that this endpoint should never be re-tried if it times out.  A blink tag might do it.

Josh
  

Nick Reid

unread,
May 11, 2017, 2:45:44 AM5/11/17
to OpenLMIS Dev

I'd second Josh's logic for a 400 error


If I were a tea pot trying to batch order coffee from multiple sources, I'd like to know I need to change something (and I should pay attention) — a 200 error means I can stop paying attention and assume all is well....


Nick Reid | nick...@villagereach.org
Friendly Neighborhood Spiderman, Information Systems Group


VillageReach Starting at the Last Mile
2900 Eastlake Ave. E, Suite 230, Seattle, WA 98102, USA

CELL: +1.510.410.0020
SKYPE: nickdotreid
www.villagereach.org




From: openlm...@googlegroups.com <openlm...@googlegroups.com> on behalf of Josh Zamor <josh....@villagereach.org>
Sent: Tuesday, May 9, 2017 1:13 PM
To: Paweł Albecki
Cc: OpenLMIS Dev

Subject: Re: [openlmis-dev] Aggregate Approvals in Malawi

Paweł Gesek

unread,
May 11, 2017, 3:33:04 AM5/11/17
to openlm...@googlegroups.com

I don't have a strong opinion, but I'm more on the side of 400 as well.


One side questions though, what type of validations errors will users usually face at this point?


Regards,
Paweł

For more options, visit https://groups.google.com/d/optout.

--

Paweł Gesek
Technical Project Manager
pge...@soldevelo.com / +48 690 020 875

Reply all
Reply to author
Forward
0 new messages