Skip to content
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

Monitor users by time since last request #541

Merged
merged 17 commits into from Dec 9, 2022
Merged

Conversation

nhoening
Copy link
Contributor

@nhoening nhoening commented Nov 27, 2022

This feature is useful for those user accounts which are expected to send/get data in automated fashion.

If they don't connect in the expected frequency, something's wrong - probably at their side. They will get alerts, just as the usual FlexMeasures operators.

± flexmeasures monitor last-seen --help                                                                
Usage: flexmeasures monitor last-seen [OPTIONS]

  Check if given users last contact (via a request) happened less than the
  allowed time ago.

  Helpful for user accounts that are expected to contact FlexMeasures
  regularly (in an automated fashion). If the last contact was too long ago,
  we send alerts via Sentry, as well as emails to monitoring mail recipients.
  The user can be informed, as well.

  The set of users can be narrowed down by roles.

Options:
  --maximum-minutes-since-last-seen INTEGER
                                  Maximal number of minutes since last
                                  request.  [required]
  --alert-users BOOLEAN           If True, also send an email to the user.
                                  Defaults to False, as these users are often
                                  bots.
  --account-role TEXT             The name of an account role to filter for.
  --user-role TEXT                The name of a user role to filter for.
  --custom-user-message TEXT      Add this message to the monitoring alert
                                  email to users (if one is sent).
  --help                          Show this message and exit.

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@coveralls
Copy link
Collaborator

coveralls commented Nov 27, 2022

Pull Request Test Coverage Report for Build 3633712562

  • 37 of 82 (45.12%) changed or added relevant lines in 6 files are covered.
  • 131 unchanged lines in 8 files lost coverage.
  • Overall coverage decreased (-0.03%) to 64.946%

Changes Missing Coverage Covered Lines Changed/Added Lines %
flexmeasures/cli/monitor.py 21 66 31.82%
Files with Coverage Reduction New Missed Lines %
flexmeasures/data/schemas/sources.py 2 81.25%
flexmeasures/data/services/scheduling.py 7 83.78%
flexmeasures/api/common/schemas/sensor_data.py 13 80.81%
flexmeasures/data/models/planning/utils.py 13 82.58%
flexmeasures/data/models/charts/defaults.py 19 52.04%
flexmeasures/data/models/charts/belief_charts.py 21 12.77%
flexmeasures/api/v3_0/sensors.py 28 70.47%
flexmeasures/cli/data_add.py 28 31.53%
Totals Coverage Status
Change from base Build 3546393110: -0.03%
Covered Lines: 6643
Relevant Lines: 9592

💛 - Coveralls

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening nhoening changed the title Monitor users by last login Monitor users by time since last request Nov 28, 2022
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening nhoening added this to the 0.12.0 milestone Nov 28, 2022
@nhoening nhoening requested a review from Flix6x November 30, 2022 14:29
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent most of my review time on understanding why a potential timezone issue I noticed wasn't causing problems. Feel free to make a separate ticket for that, because I know that timezone issues can keep you occupied for a long time, and your code doesn't actually have a problem at this point.

My only actual concern is missing opt-in / opt-out functionality. Not asking to implement that, but some documentation warning the admin about sending emails to users without them being able to opt out seems prudent.

Otherwise, only a few (other) touches to the documentation.

flexmeasures/cli/monitor.py Outdated Show resolved Hide resolved
flexmeasures/cli/monitor.py Outdated Show resolved Hide resolved
flexmeasures/cli/monitor.py Outdated Show resolved Hide resolved
flexmeasures/cli/monitor.py Show resolved Hide resolved
flexmeasures/cli/monitor.py Outdated Show resolved Hide resolved
flexmeasures/cli/monitor.py Show resolved Hide resolved
documentation/changelog.rst Outdated Show resolved Hide resolved
flexmeasures/cli/monitor.py Show resolved Hide resolved
documentation/host/error-monitoring.rst Outdated Show resolved Hide resolved
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening nhoening requested a review from Flix6x December 5, 2022 21:55
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with grammar fixes.

documentation/host/error-monitoring.rst Outdated Show resolved Hide resolved
documentation/host/error-monitoring.rst Outdated Show resolved Hide resolved
documentation/host/error-monitoring.rst Outdated Show resolved Hide resolved
@Flix6x
Copy link
Contributor

Flix6x commented Dec 6, 2022

Sidenote: Any reason why you didn't update the logic in server_now()? From your comment I figured you were enthousiastic about updating it in this PR.

@nhoening
Copy link
Contributor Author

nhoening commented Dec 6, 2022

Actually, I overlooked the logic change you proposed. Thanks!

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@Flix6x Flix6x self-requested a review December 8, 2022 13:20
… edits to messages

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening nhoening merged commit e9d6ef1 into main Dec 9, 2022
@nhoening nhoening deleted the monitor-users-by-last-login branch December 9, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants