Chiel dat 3098 create dockerized dev setup for xdiff #2

cfernhout · 2022-04-26T12:30:55Z

Adding docker-compose setup for PostgreSQL and MySQL with an automated example of how to use XDiff on the Movielens dataset.

linear · 2022-04-26T12:30:56Z

DAT-3098 Create dockerized dev setup for xDiff

https://github.com/datafold/xdiff

Create a one-click dev/test environment. I imagine a docker-compose that launches MySQL, PostgreSQL in respective containers, provisions test dataset, and allows you to run and test xDiff in one command. This will simplify further development and also make it super easy for anyone to test the tool.

…ents

erezsh

A few things I recommend changing, but all minor

erezsh · 2022-04-28T14:08:21Z

dev/prepare_db.pql

@@ -1,5 +1,6 @@
 // // Declare table & functions
 func run_sql(code) {
+    print code


I think it just adds too much clutter. If you want to see the SQL statements, you can add --print-sql when running preql.

Like

preql -m prepare_db --print-sql

--print-sql is not really workable. Because than it will spam everythin when importing the CSV. Like

Importing CSV file: 'ml-25m/ratings.csv' ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0% 0:05:32/**/;; INSERT INTO "Rating" ( "userId", "movieId", "rating", "timestamp" ) VALUES ('756', '2115', '3.5', '1167792434'), ('756', '2117', '4.0', '1167793054'), ('756', '2159', '2.5', '1167861707'), ('756', '2162', '1.5', '1167792625'), ('756', '2167', '4.5', '1167673470'), ('756', '2186', '0.5', '1167790965'), ...

I see, makes sense

erezsh · 2022-04-28T14:08:26Z

dev/setup_testenv.md

@@ -39,10 +49,10 @@ And it's ready to use!
 Example:

 ```bash
-xdiff postgres:/// Rating postgres:/// Rating_del1p -c timestamp --stats
+xdiff postgres://<uri> Rating postgres://<uri> Rating_del1 -c timestamp --stats


Technically, postgres:// is part of the uri :)

erezsh · 2022-04-28T14:09:06Z

dev/example.sh

+
+main () {
+  initialize 
+  prepaire_db


erezsh · 2022-04-28T14:09:21Z

dev/example.sh

+initialize() {
+  pip install poetry
+  poetry install
+  pip install preql==0.2.10 # Temporary due to version conflicts for runtype


0.2.11 now (or higher)

erezsh · 2022-04-28T14:10:39Z

dev/example.sh

+    unzip ml-25m.zip
+  fi
+  MYSQL_IMAGE=${MYSQL_IMAGE} docker-compose up -d
+  sleep 15 # Increase if you receive error like: `mysql.connector.errors.InterfaceError: 2013: Lost connection to MySQL server during query`


Maybe instead of sleep, check for the connection every few seconds? Sounds more robust to me.

…p-for-xdiff

sirupsen

Awesome Chiel!! Thanks for getting this here. Biggest comment to me is about whether we really need to have this whole dev pyproject.toml, rather than using the one and dev-dependencies.

sirupsen · 2022-04-28T18:42:26Z

dev/docker-compose.yml

+      command: >
+        -c work_mem=1GB
+        -c maintenance_work_mem=1GB
+        -c max_wal_size=8GB


Why was this necessary?

Yes, for the size of the movielens dataset, the defaults are too low.

sirupsen · 2022-04-28T18:42:38Z

dev/docker-compose.yml

+        --key_buffer_size=8G
+        --read_buffer_size=2G
+        --max_connections=10
+        --innodb_io_capacity=400


sirupsen · 2022-04-28T18:43:20Z

dev/example.sh

+  exit
+fi
+
+# Use a linux/arm64 docker image for MySQL when running on ARM


This worked automatically for me on the oracle images, did it not for you? Are you sure we need this?

Well, some images don't support the architectures. So you need a specific one. That is the case for this one.

sirupsen · 2022-04-28T18:43:35Z

dev/example.sh

@@ -0,0 +1,57 @@
+#!/usr/bin/env bash


I would call this dev/setup.sh or something instead—makes it a bit more obvious what it's doing

sirupsen · 2022-04-28T18:44:20Z

dev/example.sh

+  fi
+  MYSQL_IMAGE=${MYSQL_IMAGE} docker-compose up -d
+
+  until nc -z -v -w30 localhost 3306 && nc -z -v -w30 localhost 5432; do


😇 for not using a raw sleep(1)

sirupsen · 2022-04-28T18:46:26Z

dev/example.sh

+set -ex
+
+if [[ $( dirname $0 ) != "." ]]; then
+  echo "Execute from /dev folder."


Please make this execute from the root directory as vanilla script/setup.sh ❤️

I know this is partly because of preql below. The trick to not pollute the whole script is to cd in a subshell, like so:

prepare_db() { ( cd dev/ # won't pollute the main script poetry run preql -m prepare_db mysql://mysql:Password1@localhost/mysql poetry run preql -m prepare_db postgres://postgres:Password1@localhost/postgres ) }

sirupsen · 2022-04-28T18:46:45Z

dev/example.sh

@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+set -ex


❤️ I sense a fellow scarred basher

sirupsen · 2022-04-28T18:47:43Z

dev/pyproject.toml

@@ -0,0 +1,17 @@
+[tool.poetry]


I don't love this as a separate pyproject... Can't we just add these as dev-dependencies and as part of poetry install in the script install all the extras?

I'm not super familiar with Python, but I think we should follow standard conventions for contributions if possible. Presumably those are that you go to the main directory, run poetry install, and then you're in business.

sirupsen · 2022-04-28T18:47:58Z

pyproject.toml

 snowflake-connector-python = { version = "*", optional = true }

 [tool.poetry.extras]
-mysql = ["mysqlclient"]
+mysql = ["mysql-connector-python"]


👍🏻 ran into this too

sirupsen · 2022-04-28T18:51:24Z

dev/example.sh

+
+  if [ ! -f ./ml-25m/ratings.csv ]; then
+    echo "Example data not found. Downloading.."
+    wget https://files.grouplens.org/datasets/movielens/ml-25m.zip


Might we worth moving this to a test/fixtures directory or something for longer term. Don't blame you if you don't do that now though :)

Initial code copied from data-diff

…shift (#2)

cfernhout added 3 commits April 26, 2022 11:42

[DAT-3098] Dockerized dev setup for xdiff

f4cb455

[DAT-3098] Add print statements

6ecf455

[DAT-3098] Temporary workaround incompatible version required of runtype

d122b3b

cfernhout requested a review from erezsh April 26, 2022 12:36

cfernhout added 3 commits April 28, 2022 10:34

[DAT-3098] Add check correct folder and lower python version requirem…

9bcd531

…ents

[DAT-3098] Improve database config for performance

7ceac8d

[DAT-3098] Add poetry installation

6e8edd2

erezsh reviewed Apr 28, 2022

View reviewed changes

cfernhout added 2 commits April 28, 2022 18:15

"Merge branch 'master' into chiel-dat-3098-create-dockerized-dev-setu…

1961a64

…p-for-xdiff

[DAT-3098] Fix issues from testing/PR

4b6e004

sirupsen suggested changes Apr 28, 2022

View reviewed changes

sirupsen merged commit 4b6e004 into master May 5, 2022

nolar pushed a commit that referenced this pull request Apr 6, 2023

Merge pull request #2 from datafold/dev

ba4bb30

Initial code copied from data-diff

nolar pushed a commit that referenced this pull request Apr 6, 2023

Use the same select_table_schema and _normalize_table_path as Red…

d75cb4b

…shift (#2)

lokofoko mentioned this pull request Feb 1, 2024

Snowflake connection timeout after successfull loading diff results into table #862

Closed

Chiel dat 3098 create dockerized dev setup for xdiff #2

Chiel dat 3098 create dockerized dev setup for xdiff #2

Uh oh!

Conversation

cfernhout commented Apr 26, 2022

Uh oh!

linear bot commented Apr 26, 2022

Uh oh!

erezsh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sirupsen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!