Take args from JSON post data #8

abrandemuehl · 2017-04-03T15:26:12Z

Makes a change in the API to allow for multiple locations to be passed

Resolves #7

Makes a change in the API to allow for multiple locations to be passed Resolves #7

abrandemuehl · 2017-04-03T15:26:54Z

The post data should look like:

{"items": ["A1", "B2", "C3"]}

sameetandpotatoes · 2017-04-03T15:40:28Z

@abrandemuehl So would this require the merch service to make one request to /vend with all items? Should this also return an array of successes/failures so that I know which transactions specifically succeeded?

abrandemuehl · 2017-04-03T15:50:46Z

Right now, if it fails in the middle, then problems will happen. I'll update it to catch failures and return which drinks were successfully vended

abrandemuehl · 2017-04-03T15:55:56Z

How would you prefer success/failure of individual vends be indicated? I can return a combination of the index of what failed and the location.

Also, if the vend_queue feature gets put through, it'll make vending asynchronous with respect to the api requests made. We could either have the async vend queue notify the merch service at the end of the vend, or we could ditch the async vend queue for the for loop in this PR. They both work, but this one will be simpler

The machine will return the index of the vend that failed

sameetandpotatoes · 2017-04-03T16:19:13Z

Hmm, well ideally I was thinking something like this:

I send:

{
    "transaction_id": 1,
    "items": [ "A1", "B2", "C3" ]
}

and get back:

{
   "transaction_id": 1,
   "items": [
       {
             "error": null,
             "location": "A1"
       }, {
             "error": "No item present",
             "location": "B2",
       }, {
             "error": null,
             "location": "C3"
       }
   ]
}

It would be nice to get that same transaction id back so with a vend queue it's clear what transaction these items belonged to (as transaction id comes from the merch service's database).

I'm open to suggestions though.

abrandemuehl · 2017-04-03T16:35:10Z

Updated

abrandemuehl · 2017-04-04T17:46:47Z

I've added the documentation for the API to the readme

abrandemuehl · 2017-04-04T18:04:18Z

What do you think about the final result @sameetandpotatoes. I'll get this tested on the machine sometime this week.

narendasan · 2017-04-04T18:58:02Z

README.md

+Every request through the API has a token in the header that is unique to groot.
+This way, only groot can make requests to vend things.
+The token is stored in the request header.
+


Should be something like

"Requests to the device must contain a valid token in the Authorization Header for a request to be processed.
As of now the only token will be given solely to the groot merch service, so if you wish to make merch requests go through groot. "

Also rename the header field from Token to Authorization, better match with web conventions (may require change by @sameetandpotatoes on the service side)

sameetandpotatoes · 2017-04-04T19:00:10Z

Yeah this looks good to me. Before this merges though, I will need to make the corresponding change in groot-merch-service and then we should probably test that too.

rauhul

InvalidLocationError should be thrown for any location the machine physically cannot vend. If, for some reason, Groot ever needs to press another key we can create a different endpoint to do so.

rauhul

Seems good to the guy that doesn't know python 👍

rauhul · 2017-04-05T02:36:14Z

@sameetandpotatoes can you make those changes to the service

narendasan · 2017-04-05T02:39:24Z

machine_controller/app.py

@@ -45,12 +45,28 @@
 def hello_world():
    if request.headers.get('TOKEN', '') != token_value:


Change this from TOKEN to Authorization

@sameetandpotatoes Change this on service side as well ^

https://github.com/acm-uiuc/groot-merch-service/blob/7a6cf5d68565595dbe540dc92a0b6ef32e950985/models/item.rb#L49

sameetandpotatoes · 2017-04-05T06:02:12Z

acm-uiuc/groot-merch-service#11

Still needs to be tested

abrandemuehl · 2017-04-06T00:03:16Z

@narendasan Does that fix your concern?

narendasan

Lgtm

abrandemuehl · 2017-04-08T19:14:37Z

I've added the status endpoint, which will tell the service if the machine is ready to vend, as well as fixed up the errors that exist

The flask app is now multithreaded. Only one request will handle merch at a time through /vend, and the /status endpoint will return 200 if the machine is available to vend, and 503 if it is currently vending

Take args from JSON post data

c8e822a

Makes a change in the API to allow for multiple locations to be passed Resolves #7

abrandemuehl requested a review from rauhul April 3, 2017 15:27

Add vending error checking

e47adf4

The machine will return the index of the vend that failed

Add success or failure to vend endpoint

dac83a8

abrandemuehl force-pushed the json_args branch from 708e84b to a495a56 Compare April 4, 2017 17:46

abrandemuehl force-pushed the json_args branch from a495a56 to 4d0fb3b Compare April 4, 2017 17:47

Update README with API documentation

13dea4d

abrandemuehl force-pushed the json_args branch from 4d0fb3b to 13dea4d Compare April 4, 2017 18:48

narendasan reviewed Apr 4, 2017

View reviewed changes

rauhul suggested changes Apr 4, 2017

View reviewed changes

abrandemuehl added 2 commits April 4, 2017 20:07

Raise InvalidLocationError on Invalid Locations

b75a99e

Fix bounds checking

7ee4f65

rauhul approved these changes Apr 5, 2017

View reviewed changes

narendasan suggested changes Apr 5, 2017

View reviewed changes

Switch to Authorization Header for Authentication

6942484

narendasan approved these changes Apr 6, 2017

View reviewed changes

sameetandpotatoes mentioned this pull request Apr 6, 2017

Add tests, vend queue #6

Closed

Fix up errors

c624e4d

abrandemuehl force-pushed the json_args branch from b77bc22 to 944ed3f Compare April 8, 2017 19:23

Make status page return useful information

595d0ee

The flask app is now multithreaded. Only one request will handle merch at a time through /vend, and the /status endpoint will return 200 if the machine is available to vend, and 503 if it is currently vending

abrandemuehl force-pushed the json_args branch from 2bddd24 to 595d0ee Compare April 8, 2017 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Take args from JSON post data #8

Take args from JSON post data #8

abrandemuehl commented Apr 3, 2017

abrandemuehl commented Apr 3, 2017

sameetandpotatoes commented Apr 3, 2017

abrandemuehl commented Apr 3, 2017

abrandemuehl commented Apr 3, 2017

sameetandpotatoes commented Apr 3, 2017 •

edited

Loading

abrandemuehl commented Apr 3, 2017

abrandemuehl commented Apr 4, 2017

abrandemuehl commented Apr 4, 2017

narendasan Apr 4, 2017

sameetandpotatoes commented Apr 4, 2017

rauhul left a comment •

edited

Loading

rauhul left a comment

rauhul commented Apr 5, 2017

narendasan Apr 5, 2017

narendasan Apr 5, 2017

narendasan Apr 5, 2017

sameetandpotatoes commented Apr 5, 2017

abrandemuehl commented Apr 6, 2017

narendasan left a comment

abrandemuehl commented Apr 8, 2017

		@@ -45,12 +45,28 @@
		def hello_world():
		if request.headers.get('TOKEN', '') != token_value:

Take args from JSON post data #8

Are you sure you want to change the base?

Take args from JSON post data #8

Conversation

abrandemuehl commented Apr 3, 2017

abrandemuehl commented Apr 3, 2017

sameetandpotatoes commented Apr 3, 2017

abrandemuehl commented Apr 3, 2017

abrandemuehl commented Apr 3, 2017

sameetandpotatoes commented Apr 3, 2017 • edited Loading

abrandemuehl commented Apr 3, 2017

abrandemuehl commented Apr 4, 2017

abrandemuehl commented Apr 4, 2017

narendasan Apr 4, 2017

Choose a reason for hiding this comment

sameetandpotatoes commented Apr 4, 2017

rauhul left a comment • edited Loading

Choose a reason for hiding this comment

rauhul left a comment

Choose a reason for hiding this comment

rauhul commented Apr 5, 2017

narendasan Apr 5, 2017

Choose a reason for hiding this comment

narendasan Apr 5, 2017

Choose a reason for hiding this comment

narendasan Apr 5, 2017

Choose a reason for hiding this comment

sameetandpotatoes commented Apr 5, 2017

abrandemuehl commented Apr 6, 2017

narendasan left a comment

Choose a reason for hiding this comment

abrandemuehl commented Apr 8, 2017

sameetandpotatoes commented Apr 3, 2017 •

edited

Loading

rauhul left a comment •

edited

Loading