CCI-MOC / flocx-market

2 stars 9 forks source link

Manager Service Implementation #63

Closed ljmcgann closed 5 years ago

larsks commented 5 years ago

:+1: for being the first code commit that includes logging :)

tzumainn commented 5 years ago

Hi! It looks like although you've specified a periodic task, you still need to add it to a thread group, as seen here:

https://github.com/openstack/mistral/blob/master/mistral/services/expiration_policy.py#L143

I haven't found great documentation on any of this beyond what's in the code - so I might suggest just doing what esi-leap (and Ironic) does when it comes to periodic tasks.

ljmcgann commented 5 years ago

Thank you. I just got it to work. I had been looking at how nova does this all morning with no results :/

codecov-io commented 5 years ago

Codecov Report

Merging #63 into master will decrease coverage by 0.24%. The diff coverage is 94.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #63      +/-   ##
==========================================
- Coverage   97.35%   97.11%   -0.25%     
==========================================
  Files          35       39       +4     
  Lines         909     1004      +95     
==========================================
+ Hits          885      975      +90     
- Misses         24       29       +5
Impacted Files Coverage Δ
...ocx_market/tests/unit/objects/test_object_offer.py 100% <100%> (ø) :arrow_up:
flocx_market/conf/manager.py 100% <100%> (ø)
flocx_market/db/sqlalchemy/api.py 98.5% <100%> (+0.12%) :arrow_up:
flocx_market/tests/unit/api/test_app_offer.py 100% <100%> (ø) :arrow_up:
flocx_market/conf/__init__.py 100% <100%> (ø) :arrow_up:
flocx_market/objects/offer.py 100% <100%> (ø) :arrow_up:
flocx_market/tests/unit/db/sqlalchemy/test_api.py 100% <100%> (ø) :arrow_up:
flocx_market/tests/unit/manager/test_manager.py 89.47% <89.47%> (ø)
flocx_market/cmd/manager.py 90.9% <90.9%> (ø)
flocx_market/manager/service.py 91.66% <91.66%> (ø)
... and 4 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 aecbe26...6c9fdc2. Read the comment docs.

tzumainn commented 5 years ago

Okay, so it turns out there's a few more issues. I've illustrated the fixes here - https://github.com/tzumainn/flocx-market/commit/1cf22a540b073e38d2e5158c5a878e140bfd5ef3:

I'll try and get the fix for the first issue in soon!

tzumainn commented 5 years ago

I'm good with this! Just change the commit subject and the PR subject to match, and I'll merge!