search

HEPCloud / decisionengine

HEPCloud Decision Engine framework
Apache License 2.0
6 stars 26 forks source link

Metrics seems to want the channels setup to complete #610

Closed jcpunk closed 2 years ago

jcpunk commented 2 years ago

This passes the unit tests on my local box

codecov[bot] commented 2 years ago

Codecov Report

Merging #610 (3102624) into master (0b495fb) will increase coverage by 0.67%. The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #610      +/-   ##
==========================================
+ Coverage   95.95%   96.62%   +0.67%     
==========================================
  Files          47       47              
  Lines        2844     2847       +3     
  Branches      451      451              
==========================================
+ Hits         2729     2751      +22     
+ Misses         85       67      -18     
+ Partials       30       29       -1
Flag Coverage Δ
python-3.10 96.34% <85.71%> (+0.67%) :arrow_up:
python-3.6 96.22% <85.71%> (+0.67%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../decisionengine/framework/engine/DecisionEngine.py 90.85% <83.33%> (+3.35%) :arrow_up:
src/decisionengine/framework/util/sockets.py 100.00% <100.00%> (ø)
.../decisionengine/framework/engine/ChannelWorkers.py 90.78% <0.00%> (+2.63%) :arrow_up:

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 b882999...3102624. Read the comment docs.

mambelli commented 2 years ago

@shreyb @jcpunk This command is supposed to start only the web server, not to start reporting metrics. It does pass the tests though. Could it be because of logging requiring the channels? Or what else could be?

mambelli commented 2 years ago

@shreyb @jcpunk This command is supposed to start only the web server, not to start reporting metrics. It does pass the tests though. Could it be because of logging requiring the channels? Or what else could be?

Ignore that. the server logging is used here. And it's used also earlier in the code.

shreyb commented 2 years ago

The metrics collection actually starts regardless of whether the webserver is running (e.g. running de-client --metrics will still get the metrics). The --no-webserver flag will only prevent the CherryPy webserver from starting. I figured that the metrics collection itself is so lightweight we might as well have it run all the time, whether or not we actually use the metrics on any given run.