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

poetry update #2343

Merged
merged 1 commit into from
Dec 2, 2022
Merged

poetry update #2343

merged 1 commit into from
Dec 2, 2022

Conversation

tiftran
Copy link
Contributor

@tiftran tiftran commented Dec 1, 2022

No description provided.

Copy link
Collaborator

@smarnach smarnach left a comment

Choose a reason for hiding this comment

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

The issue with the migration tests appears to be unrelated to the MaxmindDB. It rather looks like one of the docker-compose containers can't connect to the Postgres database for some reason, which is surprising since the migrations are applied, so the database server is obviously running and can be contacted by some container.

When I try to run the contract tests locally, they get past the point they fail on Circle CI, but they then fail for different reasons, that look very much like they are completely out of sync with the rest of the codebase. And indeed, the last person who touched the contract tests left the company more than two years ago, and neither Mike nor anyone else touched them in the last 2.5 years. So overall I think it is safe to ignore them.

Let's get this fix merged and see whether it helps with #2341. Of course it won't fix the contract tests, but I'm ok with that. The Python tests look quite thorough to me.

Regarding the MaxMind DB, it's true that it's not possible anymore to donwload it from the former URL. The Geolite2 databases are still free in the sense that they don't cost money, but you need an account and a licence key. The database doesn't seem to be required for the tests, though, so I don't htink we need that. Moreover, it's been quite a while since the classify-client endpoint has been factored out, so we may be able to simply remove it from the Python server at this point. And we may even be able to remove the MaxMind DB from the classify-client server as well, since GCP gives us geolocation for free, so we might as well use that. None of that is urgent, of course.

@tiftran
Copy link
Contributor Author

tiftran commented Dec 2, 2022

bors merge

@bors
Copy link
Contributor

bors bot commented Dec 2, 2022

@bors bors bot merged commit 2e9f25a into mozilla:master Dec 2, 2022
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.

2 participants