start/stop/status and daemonization #1598

benjaoming · 2017-06-06T20:25:00Z

Summary

Changes the way that we initialize stuff, only running migrate and collectstatic when a change in the version is detected - this provides significant speedups of normal kolibri start command
The status command is implemented, but not broadly tested and has a McCabe complexity of 16
Running start deliberately skips status because of the above. If server is already running, it'll fail because the port is unavailable
stop behaviour is also implemented naively, just kills whatever process is in the PID file

TODO

Daemon logic
stop command
--foreground flag
Have tests been written for the new code?
Has documentation been written/updated?
New dependencies (if any) added to requirements file
Add an entry to CHANGELOG.rst
Add yourself it AUTHORS.rst if you don't appear there

Reviewer guidance

Please feel free to review code that's committed, it's intended to be merged and improved in 0.6

The WIP means that additional stuff will be added

Issues addressed

#1548

Documentation

See CLI usage spec

codecov-io · 2017-06-06T20:36:42Z

Codecov Report

Merging #1598 into develop will decrease coverage by 1.42%.
The diff coverage is 25.9%.

@@             Coverage Diff             @@
##           develop    #1598      +/-   ##
===========================================
- Coverage    73.08%   71.66%   -1.43%     
===========================================
  Files          148      149       +1     
  Lines         4882     5025     +143     
  Branches       532      552      +20     
===========================================
+ Hits          3568     3601      +33     
- Misses        1248     1357     +109     
- Partials        66       67       +1

Impacted Files	Coverage Δ
kolibri/utils/system.py	`20.51% <20.51%> (ø)`
kolibri/utils/cli.py	`30.76% <22.91%> (-6.55%)`	⬇️
kolibri/utils/server.py	`32.11% <30.37%> (+2.11%)`	⬆️

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 e5975ab...342961b. Read the comment docs.

rtibbles · 2017-06-06T20:39:15Z

Woot!

kolibri/utils/cli.py


 import django  # noqa
 from django.core.management import call_command  # noqa
 from docopt import docopt  # noqa

 from . import server  # noqa
+from .system import become_daemon  # noqa


kolibri/utils/cli.py

    if not SKIP_AUTO_DATABASE_MIGRATION:
-        call_command("migrate", interactive=False)


kolibri/utils/cli.py

    setup_logging(debug=debug)

    if not os.path.isfile(VERSION_FILE):
        _first_run()
+    else:
+        version = open(VERSION_FILE, "r").read()
+        if kolibri.__version__ != version.strip():


kolibri/utils/cli.py

+    # TODO: moved from server.start() but not sure where it should ideally be
+    # located. Question is if it should be run every time the server is started
+    # or if it depends on some kind of state change.
+    from kolibri.content.utils.annotation import update_channel_metadata_cache


kolibri/utils/cli.py

+    server.STATUS_SERVER_CONFIGURATION_ERROR: 'KA Lite server configuration error',
+    server.STATUS_PID_FILE_READ_ERROR: 'Could not read PID file',
+    server.STATUS_PID_FILE_INVALID: 'Invalid PID file',
+    server.STATUS_UNKNOW: 'Could not determine status',


kolibri/utils/server.py

+STATUS_SERVER_CONFIGURATION_ERROR = 9
+STATUS_PID_FILE_READ_ERROR = 99
+STATUS_PID_FILE_INVALID = 100
+STATUS_UNKNOW = 101


codecov-io · 2017-06-06T20:55:27Z

Codecov Report

Merging #1598 into develop will increase coverage by 0.08%.
The diff coverage is 45.17%.

@@             Coverage Diff             @@
##           develop    #1598      +/-   ##
===========================================
+ Coverage    73.35%   73.44%   +0.08%     
===========================================
  Files          148      149       +1     
  Lines         4935     5170     +235     
  Branches       545      576      +31     
===========================================
+ Hits          3620     3797     +177     
- Misses        1247     1277      +30     
- Partials        68       96      +28

Impacted Files	Coverage Δ
kolibri/deployment/default/settings/test.py	`100% <100%> (ø)`	⬆️
kolibri/utils/system.py	`32.92% <32.92%> (ø)`
kolibri/utils/server.py	`42.51% <44.32%> (+12.51%)`	⬆️
kolibri/utils/cli.py	`60.5% <58.22%> (+23.18%)`	⬆️
kolibri/deployment/default/settings/base.py	`100% <0%> (+8.33%)`	⬆️
kolibri/plugins/base.py	`72.22% <0%> (+11.11%)`	⬆️
kolibri/content/utils/channels.py	`35.59% <0%> (+11.86%)`	⬆️
kolibri/utils/conf.py	`92% <0%> (+36%)`	⬆️
... and 3 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 1afa091...34697f6. Read the comment docs.

…fect

benjaoming · 2017-06-06T21:57:05Z

Clearly in the absence of tests, this isn't very nice, but I'm trying to at least add coverage metrics on the kolibri commands invoked directly in the tox matrix... seems sort of fair :)

…ature/daemonization [ci skip]

benjaoming · 2017-06-06T22:37:33Z

Once merged, will open up issues regarding remaining work, especially:

Adding real tests (possibly undoing what I added to tox.ini just now)
Checking get_status before starting the server

rtibbles · 2017-06-07T02:47:13Z

Works, let's merge!

rtibbles

Works!

benjaoming added 2 commits June 6, 2017 19:31

Reorganize some imports in cli.py

f948886

Adding status checks and pid behavior, no daemon logic yet

342961b

benjaoming added TODO: needs gherkin update Add to our manual integration tests bug Behavior is wrong or broken TAG: new feature New user-facing feature labels Jun 6, 2017

benjaoming added this to the 0.5.0 milestone Jun 6, 2017

Daemon logic (pending a stop command)

a42be45

rtibbles reviewed Jun 6, 2017

View reviewed changes

kolibri/utils/cli.py

if not SKIP_AUTO_DATABASE_MIGRATION:

call_command("migrate", interactive=False)

This comment was marked as spam.

Sign in to view

benjaoming commented Jun 6, 2017

View reviewed changes

kolibri/utils/cli.py

setup_logging(debug=debug)

if not os.path.isfile(VERSION_FILE):

_first_run()

else:

version = open(VERSION_FILE, "r").read()

if kolibri.__version__ != version.strip():

This comment was marked as spam.

Sign in to view

rtibbles reviewed Jun 6, 2017

View reviewed changes

kolibri/utils/server.py Outdated

STATUS_SERVER_CONFIGURATION_ERROR = 9

STATUS_PID_FILE_READ_ERROR = 99

STATUS_PID_FILE_INVALID = 100

STATUS_UNKNOW = 101

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Remove PID file at exit, fix typo and rm unused kwarg from server.start

9fbaf18

benjaoming added 4 commits June 6, 2017 23:00

Release note for learningequality#1598

668de3e

Implementing the stop command

8a394f2

Use --foreground flag, remove --watch flag from usage as it has no ef…

2a0749a

…fect

An attempt of adding coverage to commands run directly

33c0a40

Merge branch 'develop' of github.com:learningequality/kolibri into fe…

3067e30

…ature/daemonization [ci skip]

benjaoming changed the title ~~[WIP] start/stop/status and daemonization~~ start/stop/status and daemonization Jun 6, 2017

benjaoming added 2 commits June 7, 2017 00:20

Use migrations because kolibri start was added to tox matrix

779b922

Remove the status command for now to deal with non-zero exit code

902acdb

Do not invoke kolibri commands in postgres tox env

34697f6

rtibbles approved these changes Jun 7, 2017

View reviewed changes

rtibbles merged commit 11f08ea into learningequality:develop Jun 7, 2017

benjaoming deleted the feature/daemonization branch June 7, 2017 12:38

benjaoming mentioned this pull request Jun 9, 2017

Start/stop/status leftovers: Add tests and more #1629

Closed

3 tasks

This was referenced Jun 20, 2017

Port KA Lite start/stop+daemon logic #1548

Closed

kolibri -h is out-of-date #139

Closed

radinamatic removed the TODO: needs gherkin update Add to our manual integration tests label Dec 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

start/stop/status and daemonization #1598

start/stop/status and daemonization #1598

benjaoming commented Jun 6, 2017 •

edited

Loading

codecov-io commented Jun 6, 2017 •

edited

Loading

rtibbles commented Jun 6, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

codecov-io commented Jun 6, 2017 •

edited

Loading

benjaoming commented Jun 6, 2017

benjaoming commented Jun 6, 2017

rtibbles commented Jun 7, 2017

rtibbles left a comment

		if not SKIP_AUTO_DATABASE_MIGRATION:
		call_command("migrate", interactive=False)

start/stop/status and daemonization #1598

start/stop/status and daemonization #1598

Conversation

benjaoming commented Jun 6, 2017 • edited Loading

Summary

TODO

Reviewer guidance

Issues addressed

Documentation

codecov-io commented Jun 6, 2017 • edited Loading

Codecov Report

rtibbles commented Jun 6, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

codecov-io commented Jun 6, 2017 • edited Loading

Codecov Report

benjaoming commented Jun 6, 2017

benjaoming commented Jun 6, 2017

rtibbles commented Jun 7, 2017

rtibbles left a comment

Choose a reason for hiding this comment

benjaoming commented Jun 6, 2017 •

edited

Loading

codecov-io commented Jun 6, 2017 •

edited

Loading

codecov-io commented Jun 6, 2017 •

edited

Loading