search

CCI-MOC / flocx-market

2 stars 9 forks source link

Implement common service to start up REST API #33

Closed DanNiESh closed 5 years ago

DanNiESh commented 5 years ago

Run cmd: flocx-market-api, then the app is start by the script cmd/start_service.py

tzumainn commented 5 years ago

I'd also suggest changing the commit message from "starting up the app" to "starting up the REST API". Remember, the REST API is just one application that will run from this code; there could be others.

tzumainn commented 5 years ago

You probably also want tests for the API. Here's the pecan version: https://github.com/CCI-MOC/esi-leap/blob/master/esi_leap/tests/api/base.py - you may want to look at how keystone tests its API.

tzumainn commented 5 years ago

Oh, and the failing CI is because you need a body for your commit message.

DanNiESh commented 5 years ago

I'd also suggest changing the commit message from "starting up the app" to "starting up the REST API". Remember, the REST API is just one application that will run from this code; there could be others.

Sure. But the REST api part is in Filip's branch, not merged in upstream. Should I take some code from Filip?

tzumainn commented 5 years ago

I think we need to decide which PR is dependent upon the other. My preference would be for this to get in first, and then for Filip's PR to rebase on top of this.

DanNiESh commented 5 years ago

Oh, and I think there's a few pep8 complaints that should probably be fixed!

Yes! I'm trying to modify the json pep8 format.

DanNiESh commented 5 years ago

One last minor comment; and I think you need to add Flask to requirements.txt. After that, I think this is good to merge!

requirements.txt, pep8, flocx-marketplace->flocx-market problems fixed.

tzumainn commented 5 years ago

Looks good!