Cron by ressource by Gabriel-Desharnais · Pull Request #175 · geopython/GeoHealthCheck

Gabriel-Desharnais · 2017-11-17T20:28:02Z

So this modification integrate the cron inside the web app. There is no need to use crontab. Also there is one cron by resource and each of them can be set at different time interval

justb4 · 2017-11-18T12:15:53Z

Thanks @Gabriel-Desharnais, this is a much requested feature from other users also described in issue #132 by @alexandreleroux . I was wondering if this could be done within a Flask webapp, especially when running in multi-process WGSI deployments like gunicorn, wouldn't we get one Scheduler per process? See e.g. this issue. As it touches the core of GHC and all existing installations and the database, please allow us some time to carefully review your PR.
But again: much appreciated.

As a first step, you may have noticed that the Travis build has failed. Mostly this is due to the Flake8 Python Coding Standards check. It is quite strict but easy to fix and test. See the Travis details. And just run find . -name "*.py" | grep -v docs | grep -v migrations | xargs flake8.

Also one of the tests failed, unclear why but your can rerun while fixing:

# install GHC with DB etc

# Load testdata into DB
python GeoHealthCheck/models.py load tests/data/fixtures.json y

# Run the healthchecks (what is normally run via cron)
python GeoHealthCheck/models.py run

Gabriel-Desharnais · 2017-11-28T17:09:30Z

That should do it

justb4 · 2017-12-08T09:24:10Z

@Gabriel-Desharnais @tomkralidis Note some discussion on Gitter on this PR. Probably better to proceed here.

justb4

Let's try take this direction. Some general remarks:

database upgrade: existing installations need to update their DBs using paver upgrade. A migration needs to be added. I have a migration script, generated when trying your PR.
some spelling errors of frequency (will indicate in code)
test_frequency use database column default i.s.o. code to always have default of 60 mins.
need to tackle the case of "multi-processing": where GHC runs as several separate processes, e.g. in Apache mod_wsgi or gunicorn. See suggestion on Gitter.
GHC_RETENTION_DAYS, why the change into days, seconds, microseconds, milliseconds, minutes, hours, weeks? Will be incompatible with existing configs. May use cron type scheduling.
flush_runs() runs once a minute, possibly once an hour or even day will do.
requirements.txt add specific version of APScheduler as dependency

justb4 · 2017-12-08T12:09:33Z

+def start_crons():
+    # Cold start evry cron of evry ressource
+    for resource in Resource.query.all():
+        freq = resource.test_frequency


Do this via setting a default value for test_frequency in Models.py and the migration script. No need to set here.

justb4 · 2017-12-08T12:10:24Z

+        'max_instances': 100000
+        })
+
+    scheduler.add_job(flush_runs, 'interval', minutes=1)


This is quite often, possibly once an hour will do.

justb4 · 2017-12-08T12:12:26Z


-GHC_RETENTION_DAYS = 30
+# days, seconds, microseconds, milliseconds, minutes, hours, weeks
+GHC_RETENTION_DAYS = (30, 0, 0, 0, 0, 0, 0)


Try to retain the existing type for GHC_RETENTION_DAYS, may expand this to a timedelta explicitly.

justb4 · 2017-12-08T12:17:19Z

+# Complete handle of old runs deletion
+def flush_runs():
+    APP = App.get_app()
+    retention_time = timedelta(*APP.config['GHC_RETENTION_DAYS'])


See earlier comments: may just calculate retention time from days, e.g. retention_time in milliseconds will be something like:
GHC_RETENTION_DAYS * 24 * 3600 * 1000

justb4 · 2017-12-08T12:18:20Z

+            <tr>
+                <th>Test frenqucy</th>
+                <td>
+                    <input type="number" id="input_resource_frequency" name="resource_frenquency_value" value="{{ resource.test_frequency }}" style="width: 100%;"/>


Spelling: resource_frenquency_value should be resource_frequency_value

justb4 · 2017-12-08T12:18:39Z

-
+
+        // Collect test_frequency
+        var new_frequency = $('input[name="resource_frenquency_value"]').val();


Spelling: resource_frenquency_value should be resource_frequency_value

justb4 · 2017-12-08T12:19:21Z

                </td>
            </tr>
+            <tr>
+              <th>{{ _('Test frenquency (minutes)') }}</th>


Spelling: frenquency should be frequency

justb4 · 2017-12-08T12:20:07Z

    owner = DB.relationship('User',
                            backref=DB.backref('username2', lazy='dynamic'))
    tags = DB.relationship('Tag', secondary=resource_tags, backref='resource')
+    test_frequency = DB.Column(DB.Integer)


add default 60 mins here

justb4 · 2018-11-05T20:54:26Z

Sorry, but closing this PR, as it has been implemented via PR #222. Thanks @Gabriel-Desharnais for the ideas in this PR. The actual implementation in #222 appeared to be more involved as GHC can run multiple processes in cases, e.g. via WSGI. Also attended via #132.

Gabriel Desharnais added 8 commits November 15, 2017 19:08

Here's just a small test to asses the situation

5163705

Add the field test_frequency to web forms

f9b0ec2

Proper integrated cron

548e8eb

Start cron at server start and use the actual time to create runs

92e6249

some stuff

f87d1bb

test

ac1ef5b

flushing works fine

b4fd42d

final step

9603404

justb4 self-requested a review November 18, 2017 12:00

justb4 added the enhancement label Nov 18, 2017

Gabriel Desharnais added 5 commits November 28, 2017 15:00

solving the model run problem

8954c73

code standart

ff7d885

code standart

77c245d

solving session problem (try)

885aef1

solving run error

329f033

solving an issue with the start of cron set to None

9228f9a

justb4 reviewed Dec 8, 2017

View reviewed changes

justb4 mentioned this pull request Dec 22, 2017

docker-compose.postgis.yml misses image entries #180

Closed

justb4 mentioned this pull request Apr 23, 2018

Add support for WPS #203

Open

justb4 mentioned this pull request Nov 5, 2018

Per resource scheduling #222

Merged

alexandreleroux mentioned this pull request Nov 5, 2018

Allow setting resource-specific testing frequency #132

Closed

justb4 closed this Nov 5, 2018



		// Collect test_frequency
		var new_frequency = $('input[name="resource_frenquency_value"]').val();

Uh oh!

Uh oh!

Conversation

Gabriel-Desharnais commented Nov 17, 2017

Uh oh!

justb4 commented Nov 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gabriel-Desharnais commented Nov 28, 2017

Uh oh!

justb4 commented Dec 8, 2017

Uh oh!

justb4 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justb4 commented Nov 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

justb4 commented Nov 18, 2017 •

edited

Loading