Send email notification when lease starts or ends soon

[DanNiESh]

Closes: #149

[larsks]

You should update your commit message and the PR description to include a link to the issue with which it is associated. This will update the issue to include a link to the commit (for mention in the commit message) and to this PR (for the mention in the PR description).

It's generally good practice for your commit message to provide some context for the changes.

If you write something like:

Closes: https://github.com/CCI-MOC/esi-leap/issues/149

Then the referenced issue will be closed automatically when this PR merges. If the PR and the issue are in the same repository, you can write:

Closes: #149

[DanNiESh]

You should update your commit message and the PR description to include a link to the issue with which it is associated. This will update the issue to include a link to the commit (for mention in the commit message) and to this PR (for the mention in the PR description).

It's generally good practice for your commit message to provide some context for the changes.

If you write something like:

Closes: https://github.com/CCI-MOC/esi-leap/issues/149

Then the referenced issue will be closed automatically when this PR merges. If the PR and the issue are in the same repository, you can write:

Closes: #149

Thank you for your advice! I've updated PR description as well as commit message with a link to the issue.

[tzumainn]

@DanNiESh this looks pretty good - I think optimally we don't run openstack commands on the command line, but I think that can be updated later once the ESI commands get an SDK.

Can you add some comments to the top of the file explaining how to run the script?

[DanNiESh]

@DanNiESh this looks pretty good - I think optimally we don't run openstack commands on the command line, but I think that can be updated later once the ESI commands get an SDK.

Can you add some comments to the top of the file explaining how to run the script?

Done!

[tzumainn]

Just a few minor text changes!

[tzumainn]

Can we add the node UUID and name as well?

[DanNiESh]

esi lease lease command doesn't have an output of node uuid. Do we want to make another call to retrieve node uuid?

[DanNiESh]

I've updated the content of email body and provided external email templates for the script.

[larsks]

I would expect this script to live in the esi_leap package directory, and to have it installed via an entry in the [entry_points] section of setup.cfg.

The default template files should live in the package directory as well (e.g., in esi_leap/templates), and you would load them from there using the importlib.resources module:

import importlib.resources

lease_expire_template = importlib.resources.files("esi_leap") / "templates" / "lease_expire_email.txt"
template_text = lease_expire_template.read_text()

You could also generate the path like this:

lease_expire_template = importlib.resources.files("esi_leap").joinpath("templates/lease_expire_email.txt")

Rather than hardcoding the package name, you could ask for __name__.split('.')[0]

(You will still want to support using alternate templates using the LEASE_CREATE_TEMPLATE and LEASE_EXPIRE_TEMPLATE environment variables.)

[DanNiESh]

updated the endpoint name to esi-leap-email-notification

[tzumainn]

Would you run esi-leap-email-notification instead? I might mention that the script would be run periodically as well.

[DanNiESh]

The script itself doesn't have scheduling mechanism. But we can make it a cronjob to run periodically. Just add the command esi-leap-email-notification in a crontab file and specify a frequency would work.

[tzumainn]

Yep, sorry, I was unclear - the documentation should be updated to say "run esi-leap-email-notification" instead of "esi_leap_query_events", right? And I think the documentation should tell the operator to set up a periodic cron job rather than just running it once.

[DanNiESh]

Makes sense! I just added this instruction in the documentation.

[tzumainn]

Looks good - thanks!