Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add logging to setup and initialize_kalite #5438

Closed
wants to merge 7 commits into from

Conversation

benjaoming
Copy link
Contributor

Summary

This also (once again) prints the server address at command line ( a regression from #5364 )

TODO

If not all TODOs are marked, this PR is considered WIP (work in progress)

  • Has documentation been written/updated?
  • Have you written release notes for the upcoming release?

Reviewer guidance

Let me know if there are problems in this approach. I don't see much of a difference, except I forced ALL of the loggers in kalite.distributed.management.commands to user a logger WITHOUT formatting, expecting their outputs to be generally targeted for command line displaying.

However, they are also subject to be printed in the server log.

Issues addressed

#5408

@benjaoming benjaoming added this to the 0.17.1 milestone Apr 10, 2017
@codecov-io
Copy link

codecov-io commented Apr 10, 2017

Codecov Report

Merging #5438 into 0.17.x will increase coverage by 0.2%.
The diff coverage is 8.47%.

Impacted file tree graph

@@            Coverage Diff            @@
##           0.17.x    #5438     +/-   ##
=========================================
+ Coverage   51.73%   51.94%   +0.2%     
=========================================
  Files         143      143             
  Lines        7494     7466     -28     
=========================================
+ Hits         3877     3878      +1     
+ Misses       3617     3588     -29
Impacted Files Coverage Δ
kalite/distributed/management/commands/setup.py 0% <0%> (ø) ⬆️
...stributed/management/commands/initialize_kalite.py 0% <0%> (ø) ⬆️
kalite/settings/base.py 88.78% <75%> (+0.1%) ⬆️
kalite/cli.py 29.03% <9.52%> (-0.11%) ⬇️
kalite/distributed/views.py 59.23% <0%> (-0.64%) ⬇️

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 a5fbd46...c331be5. Read the comment docs.

@benjaoming benjaoming force-pushed the setup-logging branch 8 times, most recently from 2266e3a to 3b9deac Compare April 12, 2017 13:52
@benjaoming
Copy link
Contributor Author

I'm wondering why Codecov does not see coverage of kalite.cli.start() which is called in the main thread during CLITestCase.test_start - closing this PR to open a fresh one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants