Postgresql in circle #4397

jeremyf · 2020-06-30T19:37:21Z

Adds optional Postgres setup for test database

cd4c970

Our CircleCI orb gives us the ENV variables this setup uses.

Adding postgresql to the engine cart build

ce1cd77

Thanks @rotated8 for the previous leg of the journey.

For both local development and CircleCI, this commit switches the default
database from Sqlite to Postgresql.

Intrepid developer, please note when next you regenerate the
.internal_test_app, you will by default be generating it to use a
Postgresql database. For now, I've included guidance on what to do in
the ./.engine_cart.yml file.

Once you've regenerated your .internal_test_app, you'll need to export
a DATABASE_URL (or update .internal_test_app/config/database.yml) to
specify how you're connecting to Postgresql.

This builds on the guidance in the Rails Guides around using a
DATABASE_URL environment variable.

Also, ensuring hyrax installs migrations in CirclCI

At a future point, we'll likely need to update the developer Wiki, but
for now, dear developer, you'll need to review the commit messages.

Changing column type [Needs Upgrade Notes]

f33c213

I do not believe this change column is likely used by adopters; It has
to do with adding permissions to an individual Sipity::Entity.

The change is necessary because the foreign key was created as a string,
but the associated primary key on the other table was an integer.

This is likely a bad copy paste job from ndlib/sipity

spec/features/create_child_work_spec.rb

spec/test_app_templates/lib/generators/test_app_generator.rb

...yrax/templates/db/migrate/20200617002029_change_sipity_entity_specific_responsibility.rb.erb

no-reply · 2020-06-30T23:58:55Z

spec/test_app_templates/lib/generators/test_app_generator.rb

@@ -5,6 +5,67 @@ class TestAppGenerator < Rails::Generators::Base
  # so the following path gets us to /path/to/hyrax/spec/test_app_templates/
  source_root File.expand_path('../../../../spec/test_app_templates/', __FILE__)

+  def conditional_postgresql_for_ci_environment
+    # For now, postgresql is something we are looking at adding to CircleCI


could all of this be handled by DATABASE_URL instead? https://edgeguides.rubyonrails.org/configuring.html#configuring-a-database

I think we could. I also believe we could then run postgresql OR sqlite locally for specs based on an ENV. That would be dreamy.

I did some further digging, and those ENV's currently come from CircleCI. Also, based on my read of that configuring a database, you'd be able to use DATABASE_URL for local development.

Perhaps this is something to bump to @rotated8 and samvera's orbs?

@jeremyf i think you can set environment variables for a job here, without requiring changes to the orb: https://github.com/samvera/hyrax/blob/master/.circleci/config.yml#L20-L22

i think adding a line like DATABASE_URL: 'postgresql://hyrax_user:hyrax_password@postgres/hyrax?pool=5' might do the trick?

@no-reply I'll give it a go on my local machine; I'd love to remove some of that conditional logic.

possibly it's okay to just hard code the CI config in multiple ENV variables as a first step, then refactor the duplication out after?

I think I'm hearing that it'd be nice if the orb exported a DATABASE_URL variable. So, samvera/samvera-circleci-orb#42

@rotated8 thanks, I'll get to hard-coding, and see about remediating that particular file. I'm a big fan of how small this PR is becoming.

The last set of commits and rebasing addresses the DATABASE_URL, and opts for the hard-coding as talked about in https://wiki.lyrasis.org/display/samvera/Samvera+Tech+Call+2020-07-15

It's much cleaner and removes several switches (though pushes people to locally use postgresql when they regen their internal test app).

oh wow this looks great! thanks for your documentation.

i appreciate the personal nature of your code comments. :)

jeremyf · 2020-07-01T21:34:09Z

So, even if these specs pass, there appears to be some "fails more than it passes" builds.

I'm going to look into the DATABASE_URL, as I think that might help someone run both a sqlite and a postgresql instance in their dummy app (based on ENV).

I also believe, at this point, that all Postgresql specific changes to the code-base have been merged via other PRs.

jeremyf · 2020-07-09T17:35:43Z

I rebased the work based on #4417

Our CircleCI orb gives us the ENV variables this setup uses.

cjcolvar

This looks good and pretty clean now!
I have only one small suggestion and a question.

.circleci/config.yml

cjcolvar · 2020-07-15T20:02:32Z

.circleci/config.yml

+      - run:
+          command: |
+            cd .internal_test_app
+            bundle install


Do we need another bundle install here? It runs at the engine level right before this.

I don't know; I'll remove and find out. Brace yourselves, rebases are coming!

I want to remove this, but without the step, things fail.

See https://app.circleci.com/pipelines/github/samvera/hyrax/3828/workflows/4011ea3f-393e-484f-a0aa-fceeaa633057/jobs/25909/steps

Thanks for trying that out. Not in scope of this PR, but I wonder if this is another place that circleci caching could be utilized.

Yeah, I'd love to not run bundle install more than is necessary. This looks like it shouldn't be necessary, but I leave it up to the next person.

no-reply · 2020-07-15T20:24:30Z

.engine_cart.yml

@@ -0,0 +1,40 @@
+# Greetings Hyrax developer, you may be surprised to see this file.


great!

these docs are helpful. i may take a pass at a docker version later on.

no-reply

🚀 🎸 ✨ 🥇

cjcolvar · 2020-07-15T20:30:09Z

.circleci/config.yml

@@ -112,6 +114,9 @@ jobs:
          ruby_version: << parameters.ruby_version >>
          bundler_version: << parameters.bundler_version >>
          project: hyrax
+      - run:
+          command: |
+            cd .internal_test_app


It looks like you lost the migration installation and db:migrate in the rebase.

jeremyf · 2020-07-15T23:00:01Z

The errors that I'm seeing indicate that I may be missing some necessary build step.

@rotated8

Thanks @rotated8 for the previous leg of the journey. For both local development and CircleCI, this commit switches the default database from Sqlite to Postgresql. Intrepid developer, please note when next you regenerate the `.internal_test_app`, you will by default be generating it to use a Postgresql database. For now, I've included guidance on what to do in the `./.engine_cart.yml` file. Once you've regenerated your `.internal_test_app`, you'll need to export a DATABASE_URL (or update `.internal_test_app/config/database.yml`) to specify how you're connecting to Postgresql. This builds on the guidance in the [Rails Guides][1] around using a DATABASE_URL environment variable. [1]:https://edgeguides.rubyonrails.org/configuring.html#configuring-a-database Also, ensuring hyrax installs migrations in CirclCI At a future point, we'll likely need to update the developer Wiki, but for now, dear developer, you'll need to review the commit messages.

I do not believe this change column is likely used by adopters; It has to do with adding permissions to an individual Sipity::Entity. The change is necessary because the foreign key was created as a string, but the associated primary key on the other table was an integer. This is likely a bad copy paste job from ndlib/sipity

cjcolvar

jeremyf force-pushed the postgresql-in-circle branch from c40bdd3 to c7a8750 Compare June 30, 2020 19:53