Feature #14507

Have the FT meeting reminder automatically avoid Fridays, Saturdays, and Sundays

Added by anonym 2017-08-30 09:36:05 . Updated 2019-06-19 12:07:39 .

Status:
Resolved
Priority:
Elevated
Assignee:
muri
Category:
Infrastructure
Target version:
Tails_3.14.1
Start date:
2017-08-30
Due date:
% Done:

50%

Feature Branch:
https://0xacab.org/muri/meeting
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

E.g. don’t allow them to happen on Fri, Sat, Sun.

Files

monthly-meeting.eml (922 B) sajolida, 2018-04-16 14:26:16

Subtasks

Related issues

Related to Tails - Feature #7763: Move our email reminders (e.g. for meetings) to Puppet In Progress 2015-03-03
Related to Tails - Feature #14503: Document Monthly Meetings fixes Rejected 2017-08-30

History

#1 Updated by anonym 2017-08-30 09:36:33

#2 Updated by anonym 2017-08-30 09:37:43

  • Assignee changed from anonym to sajolida
  • QA Check set to Info Needed

I need to know how I can proceed!

#3 Updated by anonym 2017-08-30 10:06:23

  • Assignee changed from sajolida to anonym
  • QA Check deleted (Info Needed)

#5 Updated by sajolida 2017-09-03 14:18:10

Maybe encoding this in a crontab line would be to weird. We could
instead delegate to meeting.rb the “is it time to send an email?”
decision, i.e.:

* run meeting.rb everyday, with a command-line parameter that
triggers the desired behavior, and without —date;

* either teach meeting.rb what the desired behavior is, i.e.
only send email when it thinks it should (based on the algorithm
you’ve picked); or do the computation by hand for the next 2 years,
write the result in some YAML file, and teach meeting.rb to use
this data as part of its input. With the second way,
each team who wants a meeting reminder can then build their
own schedule (including exceptions to the general rule) manually or
programmatically and feed meeting.rb with it; it requires yet
another Git repo to store the data, giving access to that repo to
the right people, and programming meeting.rb defensively enough, so
it might require a little bit more work.

#6 Updated by intrigeri 2017-09-07 06:55:33

  • Priority changed from Normal to Elevated
  • Target version changed from Tails_3.2 to Tails_3.3

(IMO this should happen in September but it can wait after the 3.2 release.)

#7 Updated by intrigeri 2017-10-04 03:54:05

I think we could (and perhaps should) update the time of the meeting in the current cronjob without waiting for the date to be computed in a more clever way: the former is currently always wrong, while the latter will only occasionally be wrong. sajolida, do you want a ticket for that?

#8 Updated by sajolida 2017-10-04 11:27:07

  • Subject changed from Implement new Monthly Meeting reminder rules to Have the monthly meeting reminder automatically avoid Fridays, Saturdays, and Sundays

I just changed the time in the template email.

#9 Updated by anonym 2017-11-15 11:30:56

  • Target version changed from Tails_3.3 to Tails_3.5

#10 Updated by intrigeri 2017-12-07 12:47:20

  • Assignee deleted (anonym)

#11 Updated by intrigeri 2018-01-04 06:51:47

  • Target version changed from Tails_3.5 to Tails_3.6

Called for help on -summit@.

#12 Updated by muri 2018-01-13 19:27:38

hi,

i’ve written a python script that can be used to send a meeting reminder. it was easier for me than to get into ruby ;) if its not useful, no worries, it was fun either way ;)

by default it shows the next meeting date.

/meeting.py 
The monthly Tails meeting in February will be on Tuesday 06. February 2018


if the meeting falls on fr,sa,su it add 3 days to the meeting date.

With -r one can set multiple days in advance, when a reminder should be sent:

<code>
./meeting.py -r 10,24
The monthly Tails meeting in February will be on Tuesday 06. February 2018
One reminder would be sent on Saturday 27. January 2018.
One reminder would be sent on Saturday 13. January 2018 - thats today!
</code>

With -mail it outputs the invitation mail on stdout, with -sendmail (and an email address via —to) it sends the invitation to that address via local smtp server. with -e/—encrypt and an email address it encrypts the output/the mail for that email address (for that, the key needs to be in the keyring of the user running the script and key in the keyring must not be expired).
If r and(send)mail are combinded, the invitation is only sent/printed to stdout if the day the script is run is a ‘reminder day’.

https://0xacab.org/muri/meeting

(update 20180115: changed -d to -r and now -d is used for setting the day the meeting should be. description above adapted accordingly)

#13 Updated by Anonymous 2018-01-19 09:53:25

  • related to Feature #14567: Investigate mobile messaging applications added

#14 Updated by Anonymous 2018-01-19 09:53:33

  • related to deleted (Feature #14567: Investigate mobile messaging applications)

#15 Updated by Anonymous 2018-01-19 10:07:44

  • related to Feature #7763: Move our email reminders (e.g. for meetings) to Puppet added

#16 Updated by Anonymous 2018-01-19 10:10:14

  • Assignee set to sajolida

We need

  1. someone who knows what the interface (input, output) of the script should be to check it satisfies our needs (sajolida?);
  2. someone to review the code (intrigeri);
  3. someone to deploy it somewhere (depends on whether we want to take this opportunity to move it to our infra (Feature #7763, and then intrigeri can do this as a volunteer) or not (and then it has to be sajolida))

So i’m now assigning this to sajolida for the time being and let him take the next steps described above.

#17 Updated by sajolida 2018-03-13 13:06:02

  • Target version changed from Tails_3.6 to Tails_3.7

#18 Updated by sajolida 2018-04-16 14:28:16

  • File monthly-meeting.eml added
  • Assignee deleted (sajolida)
  • Target version deleted (Tails_3.7)

I’m uploading here all the info about the current reminder:

0 0 24 * *  DATE=$(date --date="$(date '+\%Y-\%m-03 +1 month')" +\%F) ; ruby $HOME/meeting/meeting.rb --locale en_US.UTF-8 --template $HOME/monthly-meeting.eml --date $DATE --subject "Tails contributors meeting: $(date --date=$DATE '+\%A \%B \%d')" --from "tails-project@boum.org" --to "tails-dev@boum.org tails-project@boum.org tails-ux@boum.org tails-l10n@boum.org"

I’m not volunteering to do this coding.

#19 Updated by intrigeri 2018-04-30 14:07:25

  • Target version set to Tails_3.7
  • QA Check set to Ready for QA

u wrote:
> We need
>
> # someone who knows what the interface (input, output) of the script should be to check it satisfies our needs (sajolida?);

I’ll do that. FTR “the script” == muri’s code, that has been waiting to be reviewed since 4 months. If the script needs adjustments I’ll ask muri if he wants to do it.

#20 Updated by intrigeri 2018-04-30 14:07:33

  • Assignee set to intrigeri

#21 Updated by intrigeri 2018-05-05 13:25:03

  • Target version changed from Tails_3.7 to Tails_3.8

#22 Updated by intrigeri 2018-06-03 07:49:15

#23 Updated by intrigeri 2018-06-03 07:49:24

#24 Updated by intrigeri 2018-06-19 16:28:12

  • Target version changed from Tails_3.8 to Tails_3.9

#25 Updated by intrigeri 2018-06-28 07:37:04

  • Category set to Infrastructure

#26 Updated by intrigeri 2018-08-15 19:24:28

  • Target version changed from Tails_3.9 to Tails_3.10.1

#27 Updated by intrigeri 2018-10-20 11:06:20

  • Target version changed from Tails_3.10.1 to Tails_3.11

#28 Updated by intrigeri 2018-11-17 12:03:23

  • Status changed from Confirmed to In Progress
  • Assignee changed from intrigeri to muri
  • % Done changed from 0 to 50
  • QA Check changed from Ready for QA to Info Needed

Hi muri! I finally took time to look into this. Sorry for the delay.

This looks good to me. I see no clear blocker and we could deploy this except one: can one pass several email address via --to? If not, that’s a problem because we send our meeting reminder to a bunch of lists (and I’d rather not add one crontab entry per recipient). If that’s possible, then please document the separator in this argument’s help string.

Suggested code improvements:

  • Any reason to use Python 2?
  • I’d rather validate all the arguments in __main__: right now some part of it is done in sendmail. And once that’s fixed, sendmail can require its to argument instead of having None as a default value for it.

Feature requests and future ideas (that should not block this ticket; instead, let’s file tickets for them once we agree on something):

  • Make the template filename configurable on the command line, so this can be used for other meetings. We have one use case for it already: the FT meeting is scheduled according to exactly the same logic :)
  • Manage this with Puppet (common setup class for vcsrepo etc. + a defined resource for managing reminders for one specific meeting + a config class that uses that defined resource to schedule all our meetings): Feature #7763. If you’d rather not do it, I will (otherwise we would have to ask sajolida to deploy this by hand on his machine, which would be sad given that’s clearly Tails project infra).
  • Make it robust against “the machine is down exactly at the time the cronjob should run”. See Feature #9172 for our initial attempts and near the bottom, a systemd-based implementation idea that should work. No hurry whatsoever but would be nice to have some day :)

#29 Updated by CyrilBrulebois 2018-12-16 13:55:44

  • Target version changed from Tails_3.11 to Tails_3.12

#30 Updated by muri 2018-12-24 11:28:08

  • Assignee changed from muri to intrigeri
  • QA Check changed from Info Needed to Ready for QA

hi,

intrigeri wrote:
> Hi muri! I finally took time to look into this. Sorry for the delay.
thanks for the great review!
i’ve updated the script:

  • Bump to python3
  • Add a license text
  • run through pycodestyle
  • add comments
  • add more argument validation and do all the
    validation in main
  • fix a bug with the month, that occured if the current month is
    december
  • when the given or calcuated month is smaller than the
    current month, we now assume that this month next year is meant.
  • the -mail argument now only outputs the mail body, without
    consideration of encryption. I guess when using -mail, the
    mailcontent can be somehow piped to gpg.
  • add a —template/-t option and rename the -t/—to option to
    -a/—addresses. It is now also possible to list multiple addresses
    seperated by a comma; the default template is “template.eml”
  • the -e/—encrypt argument also takes multiple addresses, seperated
    by comma
  • fix a bug with the month not having enough days for the calculated
    meeting date to occur. the meeting will now happen in the following
    month. if the day given with -d is greater than the month has days,
    the script warns and exits.
  • add a couple of style improvements

> This looks good to me. I see no clear blocker and we could deploy this except one: can one pass several email address via --to? If not, that’s a problem because we send our meeting reminder to a bunch of lists (and I’d rather not add one crontab entry per recipient). If that’s possible, then please document the separator in this argument’s help string.

thats now possible for the -sendmail option. for the -mail option i assume that the output will be processed by some tool that can itself handle multiple recipients and the output can be piped through gpg before. that means that only -sendmail does encryption.

> Suggested code improvements:
>
> * Any reason to use Python 2?
it uses python3 now

> * I’d rather validate all the arguments in __main__: right now some part of it is done in sendmail. And once that’s fixed, sendmail can require its to argument instead of having None as a default value for it.
refactored and fixed

>
> Feature requests and future ideas (that should not block this ticket; instead, let’s file tickets for them once we agree on something):
>
> * Make the template filename configurable on the command line, so this can be used for other meetings. We have one use case for it already: the FT meeting is scheduled according to exactly the same logic :)
done

> * Manage this with Puppet (common setup class for vcsrepo etc. + a defined resource for managing reminders for one specific meeting + a config class that uses that defined resource to schedule all our meetings): Feature #7763. If you’d rather not do it, I will (otherwise we would have to ask sajolida to deploy this by hand on his machine, which would be sad given that’s clearly Tails project infra).
i can do this, i’ve taken the ticket.
(if the mails should be gpg encrypted, there would also have to be a keyring containing the keys of the recipient(s))

> * Make it robust against “the machine is down exactly at the time the cronjob should run”. See Feature #9172 for our initial attempts and near the bottom, a systemd-based implementation idea that should work. No hurry whatsoever but would be nice to have some day :)

hm. whith ‘Make it robust’ you’re referring to the script? or the cronjob? the script would have to have to store when it sent out reminders. but if we assume that the job is run as soon as the machine is up again, it would only be a problem if the machine is down the full 24h of a ‘reminder day’

#31 Updated by intrigeri 2019-01-02 21:01:13

  • Target version changed from Tails_3.12 to Tails_3.13

Hi!

Until the 3.12 release I’ll try hard to focus on things that should go in that release, so it’s unlikely I’ll have time to review this during this cycle.

Still, I’ll answer now the question you asked after moving it to Feature #7763.

#32 Updated by intrigeri 2019-01-02 21:06:30

> Still, I’ll answer now the question you asked after moving it to Feature #7763.

Scratch this, I’ll reply here and then we can file a dedicated ticket once we agree on something, as I proposed initially.

>> * Make it robust against “the machine is down exactly at the time the cronjob should run”. See Feature #9172 for our initial attempts and near the bottom, a systemd-based implementation idea that should work. No hurry whatsoever but would be nice to have some day :)

> hm. whith ‘Make it robust’ you’re referring to the script? or the cronjob?

The whole setup :)

> the script would have to have to store when it sent out reminders.

Let’s avoid writing our own job scheduler if we can :)

> but if we assume that the job is run as soon as the machine is up again, it would only be a problem if the machine is down the full 24h of a ‘reminder day’

Indeed. A setup that satisfies this assumption would be good enough IMO.

#33 Updated by intrigeri 2019-02-07 09:22:09

FTR, even though the monthly meeting is on the verge of becoming a thing of the past, this is still relevant and useful: the Foundations Team uses the same algorithm for our monthly team meetings and automated reminders would be nice :)

#34 Updated by intrigeri 2019-03-05 08:05:52

  • Feature Branch set to https://0xacab.org/muri/meeting

#35 Updated by intrigeri 2019-03-05 08:31:13

  • Assignee changed from intrigeri to muri
  • QA Check changed from Ready for QA to Dev Needed

Looks great!

Sorry again for the delay. Feel free to ignore the “nitpicking” notes below. And if you’d rather not do another iteration, feel free to let me know and I’ll fix the issues noted below, so you can focus on the Puppet side of things :)

  • Why are the -sendmail and -mail options called this way, and not --sendmail and --mail? It seems this will prevent us from having one-letter shortcuts introduced by -.
  • FYI we don’t really need encryption support at this point: none of the use cases we have require it, they’re all about meetings that we announce publically in our calendar. So feel free to drop the corresponding code if it ever makes things harder or too complex; and don’t bother adding support for a keyring in the Puppet code.
  • Nitpicking: wrt. --template, I think you could drop the manual translation to templatefile and manual handling of the default value, by passing relevant dest= and default= to parser.add_argument.
  • Nitpicking: I believe that remindertoday = True if date.today() in reminders else False could be written more simply as remindertoday = date.today() in reminders
  • I think we’ll need to make From configurable: depending on where this code is running, the SMTP server used for sending may reject the current default. And this can be useful for testing --sendmail locally.

#36 Updated by sajolida 2019-03-18 20:12:56

  • Status changed from In Progress to Rejected
  • Assignee deleted (muri)
  • Priority changed from Elevated to Normal
  • Target version deleted (Tails_3.13)

We dropped monthly meeting from the website and I deprogrammed the cron job that I had for that, see Bug #16532.
So I’m rejecting this ticket. Very sorry muri for the work done and never used :((((

#37 Updated by intrigeri 2019-03-19 07:47:56

  • Status changed from Rejected to In Progress
  • Assignee set to muri
  • Priority changed from Normal to Elevated
  • Target version set to Tails_3.14

> We dropped monthly meeting from the website and I deprogrammed the cron job that I had for that, see Bug #16532.
> So I’m rejecting this ticket. Very sorry muri for the work done and never used :((((

This was a bit hasty: as per Feature #14507#note-33, “even though the monthly meeting is on the verge of becoming a thing of the past, this is still relevant and useful: the Foundations Team uses the same algorithm for our monthly team meetings and automated reminders would be nice”.

#38 Updated by Anonymous 2019-03-19 10:24:10

  • Subject changed from Have the monthly meeting reminder automatically avoid Fridays, Saturdays, and Sundays to Have the FT meeting reminder automatically avoid Fri

#39 Updated by muri 2019-03-19 16:00:40

  • Assignee changed from muri to intrigeri
  • QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> Sorry again for the delay. Feel free to ignore the “nitpicking” notes below. And if you’d rather not do another iteration, feel free to let me know and I’ll fix the issues noted below, so you can focus on the Puppet side of things :)

I’m happy about any feedback, even if its nitpicking! (Actually most of the time nitpicking is easier to fix ;)) And I’m up to a couple of more iterations, if they’re needed ;)

> * Why are the -sendmail and -mail options called this way, and not --sendmail and --mail? It seems this will prevent us from having one-letter shortcuts introduced by -.
I don’t know ;) I have now dropped the -sendmail option because it only worked together with the --addresses options, so now if one lists addresses the script assumes that mails should be sent.
The --mail option has been renamed to --print because it just prints the template to stdout.
(not sure about most of the argument names, choosing argument names is like choosing hostnames…)

> * FYI we don’t really need encryption support at this point: none of the use cases we have require it, they’re all about meetings that we announce publically in our calendar. So feel free to drop the corresponding code if it ever makes things harder or too complex; and don’t bother adding support for a keyring in the Puppet code.
oke, its gone ;)

> * Nitpicking: wrt. --template, I think you could drop the manual translation to templatefile and manual handling of the default value, by passing relevant dest= and default= to parser.add_argument.
oh, you’re right!

> * Nitpicking: I believe that remindertoday = True if date.today() in reminders else False could be written more simply as remindertoday = date.today() in reminders
yes, looks better that way ;)

> * I think we’ll need to make From configurable: depending on where this code is running, the SMTP server used for sending may reject the current default. And this can be useful for testing --sendmail locally.
I have added a --from argument and also a --subject argument, both with defaults.

#40 Updated by intrigeri 2019-03-20 12:48:13

  • Subject changed from Have the FT meeting reminder automatically avoid Fri to Have the FT meeting reminder automatically avoid Fridays, Saturdays, and Sundays

#41 Updated by intrigeri 2019-05-01 11:25:36

  • Assignee changed from intrigeri to muri
  • QA Check changed from Ready for QA to Dev Needed

All right!

Another set of nitpicks, that again I’m happy to address myself if you’d like:

  • One concern with cd747b924906b8ca8a87f4df53887f70e27389d0: it makes sendmail() use global subject and fromaddress variables; it would be nicer to pass them as arguments, just late date and address.
  • Some doc strings & comments mention options that don’t exist anymore: “The -(send)mail options”, “neither -m|—mail”.
  • While testing, I was surprised that --print did not disable email sending. The “either print a mail to stdout or send a mail using sendmail” comment (note “either”) confirms this. Maybe s/if/elif/ in if args.addresses?

None of these are blockers and IMO this is more than good enough for deploying live, so I’ll move on to Feature #7763 (maybe not today).

I wonder if the added complexity of having this in a dedicated repo is worth it. I’m tempted to import this into puppet-tails.git. What do you think?

#42 Updated by CyrilBrulebois 2019-05-23 21:23:24

  • Target version changed from Tails_3.14 to Tails_3.15

#43 Updated by muri 2019-06-04 09:34:06

  • Status changed from In Progress to Resolved

intrigeri wrote:
> All right!
>
> Another set of nitpicks, that again I’m happy to address myself if you’d like:
>
> * One concern with cd747b924906b8ca8a87f4df53887f70e27389d0: it makes sendmail() use global subject and fromaddress variables; it would be nicer to pass them as arguments, just late date and address.
fixed in 6fc60b586c7246c8631bc520628dc9f33a222084
> * Some doc strings & comments mention options that don’t exist anymore: “The -(send)mail options”, “neither -m|—mail”.
fixed in 8ae92f1b7ac90efd59ae58c9046bec20e9104a83
> * While testing, I was surprised that --print did not disable email sending. The “either print a mail to stdout or send a mail using sendmail” comment (note “either”) confirms this. Maybe s/if/elif/ in if args.addresses?
fixed in 19b0631423bc4c47dc2c21c49e98ef9f74ff866a
>
> None of these are blockers and IMO this is more than good enough for deploying live, so I’ll move on to Feature #7763 (maybe not today).
>
> I wonder if the added complexity of having this in a dedicated repo is worth it. I’m tempted to import this into puppet-tails.git. What do you think?
Sure, i’ll just copy the files to files/meeting - I’m thus resolving this ticket and we can move further reviews to Feature #7763

#44 Updated by intrigeri 2019-06-07 10:06:49

Woohoo!

#45 Updated by anonym 2019-06-19 12:07:39

  • Target version changed from Tails_3.15 to Tails_3.14.1