-
Notifications
You must be signed in to change notification settings - Fork 183
Postgres examples #732
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
base: master
Are you sure you want to change the base?
Postgres examples #732
Conversation
01c842e to
867fc8a
Compare
d2a78e4 to
4bd1773
Compare
|
@LaurentRDC , this PR makes the example schema usable. The user must provide the instance of Postgres to target. It also generates random data to populate the database with rows. There is a safety check for the user before any change is applied to the database. |
|
@LaurentRDC please review, can I help in any way? Do you want the data generation removed? |
| putStrLn "-------------" | ||
| putStrLn "For each migration step, the sequence of SQL scripts:" | ||
| let | ||
| renderer :: BeamSqlBackendSyntax Postgres -> String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in the documentation for backendMigrationStepsScript, why not use displaySyntax, which has type Syntax -> String?
Recall that BeamSqlBackendSyntax Postgres is PgSyntax, and PgSyntax has an instance of Sql92DisplaySyntax such that displaySyntax will work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need more time to understand this, can it be moved to a follow-up issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iinsist on using displaySyntax. We want to show the appropriate, canonical way to do things in this example.
beam-postgres/examples/readme.md
Outdated
| @@ -0,0 +1,3 @@ | |||
| # Pagila example | |||
|
|
|||
| `cabal run` to see rendering of Postgres migration. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, if there are more executables, cabal run won't be able to determine which executable is implied.
From the root of the repository, you refer users to run cabal run pagila instead of cabal run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it should be cabal run pagila-print. Fixed. There is no target for cabal run.
|
Hi Peter, I started the review weeks ago, but apparently never pressed the "finish review" button. Sorry! |
`cabal run` in `pagila` package
4bd1773 to
ba070df
Compare
|
Thank you @LaurentRDC |
| -- | Given a function to convert a command to a 'String', produce a script that | ||
| -- will execute the given migration steps. Usually, the function you provide | ||
| -- eventually calls 'displaySyntax' to render the command. | ||
| backendMigrationStepsScript :: BeamSqlBackend be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you mention backendMigrationScript in the docstring for backendMigrationStepsScript, and vice-versa? This way, users can more easily navigate the documentation
| {-# LANGUAGE DerivingStrategies #-} | ||
| {-# LANGUAGE DerivingVia #-} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think deriving strategies are used anymore in this changeset
| @@ -1,4 +1,4 @@ | |||
| **Note**: The code used in this guide is in `beam-postgres/examples/Pagila/Schema/CustomMigrateExample.hs`. | |||
| **Note**: The code used in this guide is in the `pagila.cabal` package, located in `beam-postgres/examples/`. `cabal run pagila` to see a rendering of the example schema. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you mean cabal run pagila-print to see a rendering of ....
This PR adds two executables to the Postgres
Pagilaexample.cabal runprints the migration to the screen to help understanding.cabal run pagila-migrationThis applies the migration to a Postgres instance. It will overwrite the data in this Postgres instance. This is documented: https://github.com/haskell-beam/beam/blob/867fc8ae1ebbcc3b5832300a7492516902af3206/beam-postgres/examples/readme.md#pagila-example
The migration example will generate
countriesandstaffand insert them into the database after migrationV0001. ThenV0002migration is applied and thestaffare migrated toV0002.NewStaff.