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

Add parsers for Equinix maintenance notifications via email #102

Merged
merged 13 commits into from
Nov 1, 2021

Conversation

scetron
Copy link
Contributor

@scetron scetron commented Oct 26, 2021

No description provided.

Copy link
Collaborator

@chadell chadell left a comment

Choose a reason for hiding this comment

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

Nice job @scetron
Please, take care of linting

circuit_maintenance_parser/parsers/equinix.py Show resolved Hide resolved
circuit_maintenance_parser/parsers/equinix.py Outdated Show resolved Hide resolved
@glennmatthews
Copy link
Contributor

Looks like there are still some mypy linting failures for the new code.

Comment on lines +59 to +63
# for non english equinix notifications
# english section is usually at the bottom
# this skips the non english line at the top
if not self._isascii(raw_time):
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels pretty fragile to me but we can go with it for now and fix later if it proves to be an issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Unfortunately seemed to be the best way to detect the non-english characters which don't encode to ascii. If they stop including English, this would break, but than we'd have a larger problem of localization.

@glennmatthews glennmatthews merged commit 120e7ef into develop Nov 1, 2021
@chadell chadell deleted the add-equinix-provider branch November 29, 2021 10:47
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.

3 participants