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

updated module dependencies, github action for testing, and etc. #139

Merged
merged 3 commits into from
Sep 9, 2022
Merged

updated module dependencies, github action for testing, and etc. #139

merged 3 commits into from
Sep 9, 2022

Conversation

sio4
Copy link
Member

@sio4 sio4 commented Sep 7, 2022 •
edited
Loading

starting clean up for Buffalo core v1. (is also slightly related to #138)

  • 68a6d4c - passed all

    • updated module dependencies
    • updated test action to use minimum supported database versions: MySQL 5.7, Postgres 10, and Cockroach v21
    • updated CODEOWNERS
    • updated dependabot to scan only on Friday (depth 2)

  • cbe65ff - failed on cockroach/e2e (the previous version is too old)

    • adjust healthcheck interval from 10s to 5s to make the preparation faster
    • fixed dialect for e2e tests
    • fixed database ports so developers could use database containers without port conflict with local one

  • 8360d1e - passed all

    • updated local test script and makefile along with docker-compose setup (cleanup, version change, etc)
    • updated fixtures for Cockroach based on v21.1
    • updated README

@sio4 sio4 marked this pull request as draft September 7, 2022 15:24
@sio4 sio4 self-assigned this Sep 7, 2022
@sio4 sio4 added the process process or management stuffs label Sep 7, 2022
@sio4 sio4 added this to the v1 / cleanup milestone Sep 7, 2022
@sio4
Copy link
Member Author

sio4 commented Sep 9, 2022

For some reason, currently, all the e2e tests run against sqlite. should be fixed.

sio4
sio4 commented Sep 9, 2022
View reviewed changes
.github/workflows/tests.yml
@@ -25,7 +25,7 @@ jobs:
ports:
- 3307:3306
# needed because the mysql container does not provide a healthcheck
options: --health-cmd "mysqladmin ping -h localhost" --health-interval 10s --health-timeout 5s --health-retries 5
options: --health-cmd "mysqladmin ping -h localhost" --health-interval 5s --health-timeout 5s --health-retries 10
Copy link
Member Author

Choose a reason for hiding this comment

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

short interval --> fast pass

.github/workflows/tests.yml
@@ -71,15 +71,15 @@ jobs:

services:
postgres:
image: postgres:14
image: postgres:10
Copy link
Member Author

Choose a reason for hiding this comment

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

the minimum supported version of PostgreSQL is 10 currently.

.github/workflows/tests.yml
wget -qO- https://binaries.cockroachdb.com/cockroach-v2.1.0.linux-amd64.tgz | tar -xz
sudo cp -i cockroach-v2.1.0.linux-amd64/cockroach /usr/local/bin/
cockroach start --insecure --background
wget -qO- https://binaries.cockroachdb.com/cockroach-v21.1.19.linux-amd64.tgz | tar -xz
Copy link
Member Author

Choose a reason for hiding this comment

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

The currentl minimum supported version of Cockroach DB is v21.1.

.github/workflows/tests.yml
cockroach start --insecure --background
wget -qO- https://binaries.cockroachdb.com/cockroach-v21.1.19.linux-amd64.tgz | tar -xz
mv cockroach-v21.1.19.linux-amd64/* .
ln -s `pwd`/cockroach /usr/local/bin/
Copy link
Member Author

Choose a reason for hiding this comment

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

The recent version has two .so libraries so just make keep them in a single directory. However, the CLI should be in the PATH so just link it.

README.md

A Common DSL for Migrating Databases

## Create a Table

## Supported Database Engines
Copy link
Member Author

Choose a reason for hiding this comment

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

This supported version and support policy are important. The basic policy should be "supporting the minimum supported version of each engine"

database.yml
@@ -2,20 +2,20 @@ mysql:
dialect: "mysql"
database: "pop_test"
host: {{ envOr "MYSQL_HOST" "127.0.0.1" }}
port: {{ envOr "MYSQL_PORT" "3306" }}
port: {{ envOr "MYSQL_PORT" "3307" }}
Copy link
Member Author

Choose a reason for hiding this comment

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

to prevent port conflict with locally installed versions. it will make it developers easy to test Fizz without having to worry about their existing local version.

docker-compose.yml
ports:
- "3306:3306"
- "3307:3306"
healthcheck:
Copy link
Member Author

Choose a reason for hiding this comment

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

to make sure the server is ready (replacing the sleep 10 workaround)

test.sh
then
verbose="-v"
fi
args=$@
Copy link
Member Author

Choose a reason for hiding this comment

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

simply pass everything

test.sh

echo $@
COMPOSE=docker-compose
which docker-compose || COMPOSE="docker compose"
Copy link
Member Author

Choose a reason for hiding this comment

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

the recent version is distributed as a form of plugin for docker

@sio4 sio4 marked this pull request as ready for review September 9, 2022 17:45
@sio4 sio4 merged commit c681bf3 into main Sep 9, 2022
@sio4 sio4 deleted the cleanup-for-buffalo-v1 branch September 9, 2022 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sio4 sio4

Labels
process process or management stuffs
Projects
None yet
Milestone
v1.14.3
Development

Successfully merging this pull request may close these issues.
1 participant
@sio4