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

Feature - schedule snapshot task adding hours range (anacrontab like) #1233

Closed
MFlyer opened this Issue Mar 26, 2016 · 18 comments

Comments

Projects
None yet
3 participants
@MFlyer
Member

MFlyer commented Mar 26, 2016

Thinking about Rockstor running in an office (9to5 or 9to6 like mine, monday to friday/saturday), I thought it could be useful to add new options to limit / better define snapshots task execution:

  • Add hours range selection
  • Add days range selection

So, when my office it's closed and nobody is working, rockstor doesn't create unuseful snapshots

TO-DO LIST

  • js lib + integration (render values)
  • handle form on add/modify
  • db migration
  • add sched task write to rockstortab with new arg crontabwindow
  • modify scripts/scheduled_tasks/snapshot & pool scrub to check for crontab window
  • add crontab window col to scheduled tasks + beautify values
@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Mar 29, 2016

Member

Looking for suggestions @schakrava @phillxnet

First "crash"
except for js integration (found useful to create a new plugin starting from cron plugin), got a practical problem:
the new "task window for execution" param, that is going to be the second arg for snapshots schedule, accept start time (hh:mm) - end time (hh:mm) - start and end date (dayofweek)
Avail options i thought to add

  • "Always" (no time / days limits)
  • "Time window" (no day limits / time limits to from)
  • "Days window" (no time limits / day limits to from)
  • "Custom window" (time & day limits, user select for ex. from 09.00 to 17.00 on Mon-Fri)

It's possible to add alternatives, but thinking about it was causing an headache xD (every day from a particular hour without end, every day till an hour, every hour starting from a day or stopping to another one etc etc - maybe i add these 4, i like them! :) but no more )

Time window will apply only for tasks with "Every minutes/hour" cos' it make no sense to limit a task executing every day at 4:53 pm or similar.
Do you agree?

Member

MFlyer commented Mar 29, 2016

Looking for suggestions @schakrava @phillxnet

First "crash"
except for js integration (found useful to create a new plugin starting from cron plugin), got a practical problem:
the new "task window for execution" param, that is going to be the second arg for snapshots schedule, accept start time (hh:mm) - end time (hh:mm) - start and end date (dayofweek)
Avail options i thought to add

  • "Always" (no time / days limits)
  • "Time window" (no day limits / time limits to from)
  • "Days window" (no time limits / day limits to from)
  • "Custom window" (time & day limits, user select for ex. from 09.00 to 17.00 on Mon-Fri)

It's possible to add alternatives, but thinking about it was causing an headache xD (every day from a particular hour without end, every day till an hour, every hour starting from a day or stopping to another one etc etc - maybe i add these 4, i like them! :) but no more )

Time window will apply only for tasks with "Every minutes/hour" cos' it make no sense to limit a task executing every day at 4:53 pm or similar.
Do you agree?

@schakrava

This comment has been minimized.

Show comment
Hide comment
@schakrava

schakrava Mar 29, 2016

Member

Those 4 sound plenty. Your original user story is to be able to exclude scheduled tasks from running during offhours or weekends. So the UI can provide input for exclusions or inclusions. I can't imagine a good UI design atm, but my first take is that may be you can come up with html design just for 4. If you can come up with a simple design for that, then you can achieve 1-4 with one implementation. Easier said than done, good luck :)

Member

schakrava commented Mar 29, 2016

Those 4 sound plenty. Your original user story is to be able to exclude scheduled tasks from running during offhours or weekends. So the UI can provide input for exclusions or inclusions. I can't imagine a good UI design atm, but my first take is that may be you can come up with html design just for 4. If you can come up with a simple design for that, then you can achieve 1-4 with one implementation. Easier said than done, good luck :)

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Mar 30, 2016

Member

First running version (UI side) - some samples attached
UI side just missing check for hour/min start < hour/min stop day-start < day-stop

default
example1
example2

Member

MFlyer commented Mar 30, 2016

First running version (UI side) - some samples attached
UI side just missing check for hour/min start < hour/min stop day-start < day-stop

default
example1
example2

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Mar 30, 2016

Member

@MFlyer This is looking great. What do you think about adding the word "inclusive" at the end? E.g.
Week days filter - from - monday - to - friday - inclusive
That way it's obvious that the end time is not an up until but an inclusive.

Member

phillxnet commented Mar 30, 2016

@MFlyer This is looking great. What do you think about adding the word "inclusive" at the end? E.g.
Week days filter - from - monday - to - friday - inclusive
That way it's obvious that the end time is not an up until but an inclusive.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Mar 30, 2016

Member

@schakrava @phillxnet pls add all your suggestions - in particular for texts, i was arguing with myself for that xD

Member

MFlyer commented Mar 30, 2016

@schakrava @phillxnet pls add all your suggestions - in particular for texts, i was arguing with myself for that xD

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Mar 31, 2016

Member

infos

@phillxnet what do you think about this? :) Adding other text after inputs was making them go on next line

Member

MFlyer commented Mar 31, 2016

infos

@phillxnet what do you think about this? :) Adding other text after inputs was making them go on next line

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Mar 31, 2016

MFlyer added a commit to MFlyer/rockstor-jslibs that referenced this issue Mar 31, 2016

MFlyer added a commit to MFlyer/rockstor-jslibs that referenced this issue Mar 31, 2016

rockstor/rockstor-core#1233 Modified func to display selects, func to…
… return current val, block matrix toDisplay

MFlyer added a commit to MFlyer/rockstor-jslibs that referenced this issue Mar 31, 2016

rockstor/rockstor-core#1233 Corrected jquery selector for selects, co…
…rrection to getWindowType for string format, beautify content - to add checks on every start < stop

MFlyer added a commit to MFlyer/rockstor-jslibs that referenced this issue Mar 31, 2016

rockstor/rockstor-core#1233 Got good layout version, removed unuseful…
… eventhandler for ajax, needed only check function for start<stop days-times
@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Mar 31, 2016

Member

@MFlyer OK yes if it's messing with the formatting then putting in the mouse over is dandy, probably a better idea anyway to keep the clutter down. All I would suggest is that the tool tip wording be a little less wordy but great. It's nice to see this coming along.

As a general point is it OK to declare jquery-cron-window.js as GPLv2 when it was originally MIT? Sorry not up on re-licence issues, presumably you are re-licencing to cover your modifications there in.
See their: LICENSE file.
Also, and I think we are going to need @schakrava 's input on this one, maybe this library should go in the rockstor-jslibs repo, sorry but haven't yet included another js library to know for sure:
There is a section in the docs "Adding third party Javascript libraries" over at http://rockstor.com/docs/contribute.html#adding-third-party-javascript-libraries
but then it looks like you are making fairly heavy changes anyway. Just a tiny and probably wrong point anyway, I'll leave you be.

Member

phillxnet commented Mar 31, 2016

@MFlyer OK yes if it's messing with the formatting then putting in the mouse over is dandy, probably a better idea anyway to keep the clutter down. All I would suggest is that the tool tip wording be a little less wordy but great. It's nice to see this coming along.

As a general point is it OK to declare jquery-cron-window.js as GPLv2 when it was originally MIT? Sorry not up on re-licence issues, presumably you are re-licencing to cover your modifications there in.
See their: LICENSE file.
Also, and I think we are going to need @schakrava 's input on this one, maybe this library should go in the rockstor-jslibs repo, sorry but haven't yet included another js library to know for sure:
There is a section in the docs "Adding third party Javascript libraries" over at http://rockstor.com/docs/contribute.html#adding-third-party-javascript-libraries
but then it looks like you are making fairly heavy changes anyway. Just a tiny and probably wrong point anyway, I'll leave you be.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Mar 31, 2016

Member

Hi @phillxnet had the same doubts about licenses, but reading original jquery-cron.js there's a double licence (MIT + GPLv2), so... @schakrava what to do???

Third party javascript libraries: forked repo and added file there - commits are from a rockstor js library fork branch ;) - although that was not easy (after a build static no library detected so i had to manually add script scr etc etc to add_task.jst for testing
Anyway, tomorrow going on with testing & control functions ( start < stop and possibly auto reduce options. Ex. start time 10:20 stop top selects will be only 10-23:21-59 )

EDIT on Github comments "bug/strange behaviour": wrote < script src without spaces and github didn't perform an htmlencode but cut last part of my message eheh

Member

MFlyer commented Mar 31, 2016

Hi @phillxnet had the same doubts about licenses, but reading original jquery-cron.js there's a double licence (MIT + GPLv2), so... @schakrava what to do???

Third party javascript libraries: forked repo and added file there - commits are from a rockstor js library fork branch ;) - although that was not easy (after a build static no library detected so i had to manually add script scr etc etc to add_task.jst for testing
Anyway, tomorrow going on with testing & control functions ( start < stop and possibly auto reduce options. Ex. start time 10:20 stop top selects will be only 10-23:21-59 )

EDIT on Github comments "bug/strange behaviour": wrote < script src without spaces and github didn't perform an htmlencode but cut last part of my message eheh

MFlyer added a commit to MFlyer/rockstor-jslibs that referenced this issue Apr 2, 2016

rockstor/rockstor-core#1233 added auto select control from hours mins…
… and days start to autoselect stops - day_stop>=day_start, hour,days_stop > hour,days_start with check on minutes 59 value - to add warning on stops change becoming < starts
@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Apr 2, 2016

Member

Some news to @schakrava & @phillxnet

Days: stop day always get auto equal to start day, then user select his day
dayinit daytoday

Hours & Mins:

  • on start hour change auto stop hour = start hour (if star min already == 59 auto correct stop hour & mins to go next hour min 0)
  • on start minute change auto stop minute = start min+1 (if start min == 59 stop hour+1 & stop min 0)

timeinit
timehourchange
timehourchanged Stop hour changing on start hour changed

timeminutechangedStop mins changing to +1 on start mins changed

nexthourStart mins == 59, move 1 min ahead that means next hour min 0

Stop hour, mins & days: decided not to add a control to force stops > stars, but just a warning for the users.
So doing users have full possibilites: for example a task from 09.00 to 10.30 on Mon-Fri will be ok without warning and a task from 19.30 to 08.30 on Mon-Fri or Fri-Tue will be always ok but with a warning (something like "Hey guy, you're not using the usual time sequence 00.00 to 23.59 Mon Fry, it will be ok, are you sure what your doing?")

Last check to add: hide task execution windows for task != to minutes or hours (it makes no sense to add it on a task running every 3rd of april at 3.54 or similar)

Flyer/Mirko

Member

MFlyer commented Apr 2, 2016

Some news to @schakrava & @phillxnet

Days: stop day always get auto equal to start day, then user select his day
dayinit daytoday

Hours & Mins:

  • on start hour change auto stop hour = start hour (if star min already == 59 auto correct stop hour & mins to go next hour min 0)
  • on start minute change auto stop minute = start min+1 (if start min == 59 stop hour+1 & stop min 0)

timeinit
timehourchange
timehourchanged Stop hour changing on start hour changed

timeminutechangedStop mins changing to +1 on start mins changed

nexthourStart mins == 59, move 1 min ahead that means next hour min 0

Stop hour, mins & days: decided not to add a control to force stops > stars, but just a warning for the users.
So doing users have full possibilites: for example a task from 09.00 to 10.30 on Mon-Fri will be ok without warning and a task from 19.30 to 08.30 on Mon-Fri or Fri-Tue will be always ok but with a warning (something like "Hey guy, you're not using the usual time sequence 00.00 to 23.59 Mon Fry, it will be ok, are you sure what your doing?")

Last check to add: hide task execution windows for task != to minutes or hours (it makes no sense to add it on a task running every 3rd of april at 3.54 or similar)

Flyer/Mirko

MFlyer added a commit to MFlyer/rockstor-jslibs that referenced this issue Apr 2, 2016

MFlyer added a commit to MFlyer/rockstor-jslibs that referenced this issue Apr 2, 2016

rockstor/rockstor-core#1233 added auto warning on stops > starts, som…
…e style changes for select tables, minor corrections to value reading/assign
@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer
Member

MFlyer commented Apr 2, 2016

@schakrava

This comment has been minimized.

Show comment
Hide comment
@schakrava

schakrava Apr 3, 2016

Member

Looks very impressive!!!!

Is there way to go one step beyond warning on wrong input? the warning message is great but what happens if they ignore and submit anyway?

Feel free to open the pr soon as you think its ready to be tested to some degree.

regarding js libraries, i'll take care of it during merge time.

Member

schakrava commented Apr 3, 2016

Looks very impressive!!!!

Is there way to go one step beyond warning on wrong input? the warning message is great but what happens if they ignore and submit anyway?

Feel free to open the pr soon as you think its ready to be tested to some degree.

regarding js libraries, i'll take care of it during merge time.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Apr 3, 2016

Member

Hi @schakrava, let see prev video message:

Stop hour, mins & days: decided not to add a control to force stops > stars, but just a warning for the users.
So doing users have full possibilites: for example a task from 09.00 to 10.30 on Mon-Fri will be ok without warning and a task from 19.30 to 08.30 on Mon-Fri or Fri-Tue will be always ok but with a warning (something like "Hey guy, you're not using the usual time sequence 00.00 to 23.59 Mon Fry, it will be ok, are you sure what your doing?")

We accept every data, you'll be able to add limits for example from 9 to 17 Mon Fri (and we call this "conventional") or from 17 to 6 Sun Tue ("unconventional" time format, because hours mins days aren't clockwise).
First case snapshots get created during 9-17 and every day from mon to fri, second case snapshots get created only if it's >17 and <6am only from sunday to tuesday.
That one showing it's just a warning that sounds like "Hey, look what are you doing, if that is your desired timinig ok, else change it, but we accept it in this format too"

Info needed to go on: i'm going to pr js library today (think so, just some checks), than i'll work on rockstore/core to manage data post, snapshots scripts control etc etc.

What about models??? Finally I've decided to add a field to rockstor db and not to use the same field of crontab (2 different fields = clear schema) and have to perform a migration (VM still on 12-05). What's is better? VM Update to latest version and then coding or just don't care about new version and perform db migration?

Flyer

Member

MFlyer commented Apr 3, 2016

Hi @schakrava, let see prev video message:

Stop hour, mins & days: decided not to add a control to force stops > stars, but just a warning for the users.
So doing users have full possibilites: for example a task from 09.00 to 10.30 on Mon-Fri will be ok without warning and a task from 19.30 to 08.30 on Mon-Fri or Fri-Tue will be always ok but with a warning (something like "Hey guy, you're not using the usual time sequence 00.00 to 23.59 Mon Fry, it will be ok, are you sure what your doing?")

We accept every data, you'll be able to add limits for example from 9 to 17 Mon Fri (and we call this "conventional") or from 17 to 6 Sun Tue ("unconventional" time format, because hours mins days aren't clockwise).
First case snapshots get created during 9-17 and every day from mon to fri, second case snapshots get created only if it's >17 and <6am only from sunday to tuesday.
That one showing it's just a warning that sounds like "Hey, look what are you doing, if that is your desired timinig ok, else change it, but we accept it in this format too"

Info needed to go on: i'm going to pr js library today (think so, just some checks), than i'll work on rockstore/core to manage data post, snapshots scripts control etc etc.

What about models??? Finally I've decided to add a field to rockstor db and not to use the same field of crontab (2 different fields = clear schema) and have to perform a migration (VM still on 12-05). What's is better? VM Update to latest version and then coding or just don't care about new version and perform db migration?

Flyer

MFlyer added a commit to MFlyer/rockstor-jslibs that referenced this issue Apr 3, 2016

rockstor/rockstor-core#1233 Added latest mods - task exec window hide…
…/show based on cron task values - ready to PR

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 4, 2016

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 4, 2016

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Apr 4, 2016

Member

to @schakrava - Coding stop on migration matter 😕

See latest commit for task_def mods (migration task doesn't raise any error) MFlyer@40e8687

On migration i get this following response and I'm not sure if safe (looked at task model and actually task.start is togheter an index key field and a nullable field - or at least a field that on empty text uses null instead of empty - ?!?, so the auto correction proposed makes sense)

Waiting for you to procede with migration and finishing code 😊

[root@rockstone models]# /opt/rockstor/bin/django schemamigration smart_manager --auto
 + Added field crontabwindow on smart_manager.TaskDefinition
 ? The field 'Task.start' does not have a default specified, yet is NOT NULL.
 ? Since you are making this field nullable, you MUST specify a default
 ? value to use for existing rows. Would you like to:
 ?  1. Quit now.
 ?  2. Specify a one-off value to use for existing columns now
 ?  3. Disable the backwards migration by raising an exception; you can edit the migration to fix it later
 ? Please select a choice: 3
 ~ Changed field start on smart_manager.Task
Created 0008_auto__add_field_taskdefinition_crontabwindow__chg_field_task_start.py. You can now apply this migration with: ./manage.py migrate smart_manager
Member

MFlyer commented Apr 4, 2016

to @schakrava - Coding stop on migration matter 😕

See latest commit for task_def mods (migration task doesn't raise any error) MFlyer@40e8687

On migration i get this following response and I'm not sure if safe (looked at task model and actually task.start is togheter an index key field and a nullable field - or at least a field that on empty text uses null instead of empty - ?!?, so the auto correction proposed makes sense)

Waiting for you to procede with migration and finishing code 😊

[root@rockstone models]# /opt/rockstor/bin/django schemamigration smart_manager --auto
 + Added field crontabwindow on smart_manager.TaskDefinition
 ? The field 'Task.start' does not have a default specified, yet is NOT NULL.
 ? Since you are making this field nullable, you MUST specify a default
 ? value to use for existing rows. Would you like to:
 ?  1. Quit now.
 ?  2. Specify a one-off value to use for existing columns now
 ?  3. Disable the backwards migration by raising an exception; you can edit the migration to fix it later
 ? Please select a choice: 3
 ~ Changed field start on smart_manager.Task
Created 0008_auto__add_field_taskdefinition_crontabwindow__chg_field_task_start.py. You can now apply this migration with: ./manage.py migrate smart_manager
@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Apr 4, 2016

Member

Found my way without --auto and this + migrate next
/opt/rockstor/bin/django schemamigration smart_manager --add-field TaskDefinition.crontabwindow

newdb

@schakrava Now i just need to know if Rockstor still need migration file (to add on git) or not

Going on with code, hope to close it before friday!

Member

MFlyer commented Apr 4, 2016

Found my way without --auto and this + migrate next
/opt/rockstor/bin/django schemamigration smart_manager --add-field TaskDefinition.crontabwindow

newdb

@schakrava Now i just need to know if Rockstor still need migration file (to add on git) or not

Going on with code, hope to close it before friday!

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 4, 2016

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 4, 2016

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 4, 2016

MFlyer added a commit to MFlyer/rockstor-jslibs that referenced this issue Apr 5, 2016

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 5, 2016

rockstor/rockstor-core#1233 Added control on add_scheduled_task.js ne…
…w crontabwindow val - full backward compatible with old tasks - on first edit and save old tasks get exec window to always, if none different specified

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 5, 2016

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Apr 5, 2016

Member

Update to do list

Member

MFlyer commented Apr 5, 2016

Update to do list

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 7, 2016

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 7, 2016

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 7, 2016

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 7, 2016

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Apr 7, 2016

Member

Hi guys @schakrava @phillxnet , got good news: added all we need to limits tasks execution and just missing scheduled tasks page "beautify" to show crontab window execution column

Performed tests with "conventional" time format (ex.: task from 9 to 17 mon - fri), with "unconventional" too (ex. task from 23 to 6 mon - fri or 9 to 15 fri - tue, etc etc) and it seems all feeling good 😃

Note for @schakrava : choosed to add crontabwindow.py as a separate module cos' both snapshot and scrub task share same code (to avoid doubled code!)

Personal consideration: not really sure about utility of crontab window over scrubs (usually you set 1 scrub/day on a specified time), but we've got it too - everyone will be free to decide

Updated first message with task list, hope to close tomorrow, as planned

Flyer/Mirko

P.S.: commits sent testing git new gpg signing option, nice!

Member

MFlyer commented Apr 7, 2016

Hi guys @schakrava @phillxnet , got good news: added all we need to limits tasks execution and just missing scheduled tasks page "beautify" to show crontab window execution column

Performed tests with "conventional" time format (ex.: task from 9 to 17 mon - fri), with "unconventional" too (ex. task from 23 to 6 mon - fri or 9 to 15 fri - tue, etc etc) and it seems all feeling good 😃

Note for @schakrava : choosed to add crontabwindow.py as a separate module cos' both snapshot and scrub task share same code (to avoid doubled code!)

Personal consideration: not really sure about utility of crontab window over scrubs (usually you set 1 scrub/day on a specified time), but we've got it too - everyone will be free to decide

Updated first message with task list, hope to close tomorrow, as planned

Flyer/Mirko

P.S.: commits sent testing git new gpg signing option, nice!

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 7, 2016

#1233 Modified task_defs.jst and scheduled_tasks.js to render crontab…
… window col - little javascript func required as inline in scheduled_tasks.js to avoid new pull to rocstor jslib
@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Apr 7, 2016

Member

Little preview on tasks list interface ;)

immagine

Finished coding, sent PR

Member

MFlyer commented Apr 7, 2016

Little preview on tasks list interface ;)

immagine

Finished coding, sent PR

schakrava added a commit to rockstor/rockstor-jslibs that referenced this issue Apr 24, 2016

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Apr 25, 2016

Member

Branch merged and avail with Rockstor 3.8.13-03 - Issue closed

Member

MFlyer commented Apr 25, 2016

Branch merged and avail with Rockstor 3.8.13-03 - Issue closed

@MFlyer MFlyer closed this Apr 25, 2016

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