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 (2) #5441

Merged
merged 8 commits into from Apr 12, 2017

Conversation

Projects
None yet
2 participants
@benjaoming
Member

benjaoming commented Apr 12, 2017

Taking over from #5438

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 12, 2017

@benjaoming benjaoming self-assigned this Apr 12, 2017

@benjaoming benjaoming added the has PR label Apr 12, 2017

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Apr 12, 2017

Codecov Report

Merging #5441 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    #5441     +/-   ##
=========================================
+ 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 Δ
...stributed/management/commands/initialize_kalite.py 0% <0%> (ø) ⬆️
kalite/distributed/management/commands/setup.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...17e2754. Read the comment docs.

codecov-io commented Apr 12, 2017

Codecov Report

Merging #5441 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    #5441     +/-   ##
=========================================
+ 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 Δ
...stributed/management/commands/initialize_kalite.py 0% <0%> (ø) ⬆️
kalite/distributed/management/commands/setup.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...17e2754. Read the comment docs.

@benjaoming

This comment has been minimized.

Show comment
Hide comment
@benjaoming

benjaoming Apr 12, 2017

Member

Doesn't seem like it's possible to trick Codecov into a new coverage report, leaving it here.

Member

benjaoming commented Apr 12, 2017

Doesn't seem like it's possible to trick Codecov into a new coverage report, leaving it here.

@benjaoming benjaoming merged commit 2d04de9 into learningequality:0.17.x Apr 12, 2017

@benjaoming benjaoming removed the has PR label Apr 12, 2017

@benjaoming benjaoming deleted the benjaoming:setup-logging branch Apr 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment