search

CCI-MOC / flocx-market

2 stars 9 forks source link

Implemented Bid API #31

Closed fvukelic closed 5 years ago

fvukelic commented 5 years ago

Wrote some additional tests for the Bid API and added the necessary endpoints.

tzumainn commented 5 years ago

@fvukelic can you rebase on top of master to pull in the changes from https://github.com/CCI-MOC/flocx-market/pull/33 ? That should pull in the updates to have the rest api run from a script defined by an entrypoint. You can copy that pattern to create a script for initializing the database, similar to https://github.com/openstack/ironic/blob/master/ironic/cmd/dbsync.py

larsks commented 5 years ago

If you rebase instead of merge, you'll get rid of that extra commit and the commit checks will pass (leaving you with the pep8 failures).

codecov-io commented 5 years ago

Codecov Report

Merging #31 into master will decrease coverage by 7.44%. The diff coverage is 67.02%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
- Coverage   89.86%   82.41%   -7.45%     
==========================================
  Files          26       30       +4     
  Lines         572      597      +25     
==========================================
- Hits          514      492      -22     
- Misses         58      105      +47
Impacted Files Coverage Δ
flocx_market/tests/unit/models/test_bid_unit.py 100% <100%> (ø)
flocx_market/api/app.py 100% <100%> (+5.55%) :arrow_up:
flocx_market/db/sqlalchemy/bid_model.py 100% <100%> (ø)
flocx_market/api/bid.py 35.48% <35.48%> (ø)
flocx_market/db/sqlalchemy/bid_api.py 64.51% <64.51%> (ø)
flocx_market/api/offer.py 33.33% <0%> (-66.67%) :arrow_down:
flocx_market/cmd/api.py 0% <0%> (ø) :arrow_up:
flocx_market/tests/unit/api/test_app.py 100% <0%> (ø) :arrow_up:
flocx_market/tests/unit/conftest.py
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2be1530...f0a353d. Read the comment docs.

fvukelic commented 5 years ago

can we merge this @larsks @tzumainn?

larsks commented 5 years ago

@fvukelic just noticed this still has a /bids endpoint, which we had talked about removing since it's contrary to the spec document.

fvukelic commented 5 years ago

@fvukelic just noticed this still has a /bids endpoint, which we had talked about removing since it's contrary to the spec document.

hmm that is really weird, I don't have it locally, I'll remove it @larsks

larsks commented 5 years ago

Maybe I was looking at an older version of the pr (I was looking locally)? If so, sorry for the noise!

fvukelic commented 5 years ago

The only problem I'm seeing is that I have each model and model api separated into separate files, but @tzumainn merged a PR with having them all in a single file like on most Openstack projects, so this is causing a merge conflict. Which approach should we take here @tzumainn @larsks? When I asked 2-3 weeks ago, you said it was fine if we keep them separated, let me know if this changed @tzumainn and I'll modify my PR. Thanks!

larsks commented 5 years ago

I don't think we need to update this PR with those changes yet. Just resolve the conflict in flocx_market/db/sqlalchemy/offer_api.py (by deleting if after a rebase), and I think we're good to merge.

fvukelic commented 5 years ago

@larsks, just rebased and Travis likes it