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

Pin Mariadb to 10.8.2 #4279

Merged
merged 1 commit into from
Aug 2, 2022
Merged

Conversation

thornycrackers
Copy link
Contributor

When trying to run docker-compose up the mysql container would never come up. The logs mentioned "Can't initialize timers" and the first result that came up was MariaDB/mariadb-docker#434 which has this comment MariaDB/mariadb-docker#434 (comment) which mentions pinning to 10.8.2. After pinning to that version everything seemed to work fine.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 25, 2022

Test Results

       35 files         35 suites   1h 10m 7s ⏱️
  8 863 tests   7 084 ✔️ 1 779 💤 0
32 469 runs  25 693 ✔️ 6 776 💤 0

Results for commit b8ccc69.

♻️ This comment has been updated with latest results.

@cpcloud cpcloud added this to the 3.2.0 milestone Jul 25, 2022
@gforsyth
Copy link
Member

Hey @thornycrackers -- thanks for the PR! I can't reproduce the issue with 10.8.3 on my end. I don't see a problem with pinning to 10.8.2 until whatever is going on is fixed, though.

What version of docker are you using?

@cpcloud
Copy link
Member

cpcloud commented Jul 30, 2022

@thornycrackers Thanks for the PR!

Can you adjust the commit message to ci: pin mariadb to 10.8.2? After that this'll be good to merge!

@cpcloud cpcloud added ci Continuous Integration issues or PRs dependencies Issues or PRs related to dependencies labels Jul 30, 2022
@thornycrackers
Copy link
Contributor Author

Updated the commit message! The version of docker I'm using is:

Docker version 20.10.9, build v20.10.9

I know it's a bit old but my NixOS flake input is set to nixpkgs/nixos-21.11

@cpcloud
Copy link
Member

cpcloud commented Aug 2, 2022

It might be related to your nixos version, but it's fine for us to pin to 10.8.2 for now. At what point should we try to bump it again?

@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Merging #4279 (b8ccc69) into master (f93752b) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4279      +/-   ##
==========================================
+ Coverage   92.57%   92.60%   +0.03%     
==========================================
  Files         179      179              
  Lines       20186    20228      +42     
  Branches     2887     2892       +5     
==========================================
+ Hits        18687    18732      +45     
+ Misses       1134     1132       -2     
+ Partials      365      364       -1     
Impacted Files Coverage Δ
ibis/backends/base/sql/registry/main.py 89.65% <0.00%> (-1.19%) ⬇️
ibis/backends/base/sql/compiler/query_builder.py 96.68% <0.00%> (-0.69%) ⬇️
ibis/backends/duckdb/__init__.py 97.61% <0.00%> (ø)
ibis/expr/operations/relations.py 97.81% <0.00%> (+<0.01%) ⬆️
ibis/expr/types/relations.py 93.53% <0.00%> (+0.04%) ⬆️
ibis/backends/pyspark/compiler.py 96.10% <0.00%> (+0.04%) ⬆️
ibis/backends/pandas/execution/generic.py 91.36% <0.00%> (+0.18%) ⬆️
ibis/backends/impala/__init__.py 86.14% <0.00%> (+0.21%) ⬆️
ibis/backends/base/sql/compiler/base.py 96.96% <0.00%> (+0.24%) ⬆️
ibis/backends/base/sql/alchemy/query_builder.py 92.46% <0.00%> (+0.43%) ⬆️
... and 3 more

@cpcloud cpcloud merged commit 22a587b into ibis-project:master Aug 2, 2022
@thornycrackers
Copy link
Contributor Author

Ehhhh, I don't know what your goals for the "dev" experience are. From my experience it's pretty easy to update docker so it's not unreasonable to say it's a requirement. Maybe after the next release just unpin it and say that old docker versions aren't supported for dev.

@cpcloud
Copy link
Member

cpcloud commented Aug 2, 2022

To be honest we haven't had a lot of people show up with different docker versions. I think many contributors don't run the backend tests because it can seem like a big lift.

We've spent some time over the last year trying to reduce the number of steps required to run the backend test suite. In particular @anjakefala recently landed a wonderful PR that removed the requirement to load data manually before running tests.

We don't have any explicit goals for the dev experience beyond:

  1. wide support for environment tooling
  2. a small number of commands needed to run tests, right now it's docker compose up --wait && pytest -m some_backend

We don't have a strong requirement on which mariadb version to run against, so your suggestion sounds fine to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous Integration issues or PRs dependencies Issues or PRs related to dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants