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

Remove mongodb #1037

Merged
merged 32 commits into from
Feb 9, 2023
Merged

Remove mongodb #1037

merged 32 commits into from
Feb 9, 2023

Conversation

rem1niscence
Copy link
Contributor

@rem1niscence rem1niscence commented Feb 6, 2023

Remove all references of mongodb to only keep the postgres db as the only source of truth. This is done by

  • Removing anywhere where mongo was used
  • Removing fallback code from phd
  • Modify all tests that needed to have some data from a db to use a HTTP mock to retrieve PHD results
  • Add a small function to phd to return HTTP error objects in case the HTTP call fails

Although I tried my best to not modify any logic, a small change had to be done to the loadBlockchains function to return the expected Error response, besides that no other change was made to any logic

@rem1niscence rem1niscence requested a review from a team as a code owner February 6, 2023 14:37
@height
Copy link

height bot commented Feb 6, 2023

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@rem1niscence rem1niscence marked this pull request as draft February 6, 2023 14:37
@rem1niscence rem1niscence marked this pull request as ready for review February 8, 2023 20:03
Copy link
Contributor

@commoddity commoddity left a comment

Choose a reason for hiding this comment

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

These changes all look good to me. Thanks man.

@@ -275,12 +266,13 @@ describe('Pocket relayer service (unit)', () => {
})

it('throws an error when loading an invalid blockchain', async () => {
axiosMock.reset()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed here? Doesn't this happen after each test case already with the added code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes and that's exactly why I put it there, to remove the blockchain response so it returns 404 and mimics the not found test

@rem1niscence rem1niscence merged commit 333fb8e into develop Feb 9, 2023
@rem1niscence rem1niscence deleted the remove-mongo branch February 9, 2023 17:16
This was referenced Feb 9, 2023
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