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.

Sign up for GitHub

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

Jump to bottom

Disable GE certificate verification and refactor (#3831) #3914

Merged
merged 3 commits into from
Mar 31, 2022
Merged

Disable GE certificate verification and refactor (#3831) #3914

merged 3 commits into from
Mar 31, 2022

Conversation

kruschk
Copy link
Contributor

@kruschk kruschk commented Mar 27, 2022

  • Disable certificate verification to fix GE production parser down #3831. This is a temporary measure and should be reverted as soon as possible (I left to-do comments in the code to this effect).
  • Refactor the parser.

@kruschk
fix(parser): disable GE cert verification (electricitymaps#3831)
59b108b 
Disable certificate verification in all network requests to fix electricitymaps#3831.
This is a temporary measure until the server-side certificate issues are
resolved.
@kruschk
refactor(parser): tidy up the GE parser
972e970
madsnedergaard
madsnedergaard approved these changes Mar 30, 2022
View reviewed changes
Copy link
Member

@madsnedergaard madsnedergaard left a comment

Choose a reason for hiding this comment

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

It's a bit hard to review this PR, but the overall the changes and refactoring looks good to me! 💪

@madsnedergaard madsnedergaard enabled auto-merge (squash) March 30, 2022 09:15
@kruschk
Copy link
Contributor Author

kruschk commented Mar 30, 2022

I was trying to disable certificate verification in the first commit and then refactor in the second, but I didn't do a very good job of it; a little bit of the refactor bled into the first. Sorry about that! If you like, I can clean up the commits to better reflect what I originally intended.

@madsnedergaard
Copy link
Member

I was trying to disable certificate verification in the first commit and then refactor in the second, but I didn't do a very good job of it; a little bit of the refactor bled into the first. Sorry about that! If you like, I can clean up the commits to better reflect what I originally intended.

Nah it's fine, I looked through the refactored code and it's much better! :)

@madsnedergaard madsnedergaard merged commit 28b6412 into electricitymaps:master Mar 31, 2022
@FelixDQ FelixDQ mentioned this pull request Jun 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@madsnedergaard madsnedergaard madsnedergaard approved these changes

Assignees
No one assigned
Labels
parser
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GE production parser down
2 participants
@kruschk @madsnedergaard