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

Update docker infra and fix image building and publishing in CI #60

Merged
merged 1 commit into from
Apr 9, 2020

Conversation

mjgiarlo
Copy link
Member

@mjgiarlo mjgiarlo commented Apr 9, 2020

Why was this change made?

To update the docker infrastructure for suri-rails and get the image build working again.

Was the documentation (README, DevOpsDocs, wiki, openapi.yml) updated?

Yes.

Does this change affect how this application integrates with other services?

Yes, will test in argo, dor-services-app, and hydrus.

@mjgiarlo mjgiarlo force-pushed the fix-image-build branch 2 times, most recently from 29f7866 to a762f3c Compare April 9, 2020 19:18
@mjgiarlo mjgiarlo marked this pull request as ready for review April 9, 2020 19:26
Copy link
Contributor

@ndushay ndushay left a comment

Choose a reason for hiding this comment

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

LGTM but i didn't run in locally or anything. Just read/skimmed it.

Comment on lines -50 to -55
- restore_cache:
keys:
- suri-rails-dependencies-{{ checksum "Gemfile.lock" }}
# fallback to using the latest cache if no exact match is found
- suri-rails-dependencies-

Copy link
Contributor

Choose a reason for hiding this comment

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

we don't want to cache because ...

Copy link
Member Author

Choose a reason for hiding this comment

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

@ndushay Good question! This caching is messed up in a few ways. First, it uses different keys for restore and save (dor-services vs. suri-rails). Second, I don't believe the cache is persisted across runs. So, basically, I think this is copypasta from some of our other repos where caching has been useful (e.g., in our GoLang and React work), and I'm removing it from Ruby/Rails projects whenever I see it.

@mjgiarlo mjgiarlo changed the title Update docker infra and fix image building and publishing in CI [HOLD] Update docker infra and fix image building and publishing in CI Apr 9, 2020
@mjgiarlo
Copy link
Member Author

mjgiarlo commented Apr 9, 2020

marking this as hold until I add some request specs that flag errors like sul-dlss-deprecated/hydrus#415

Includes:
* Return errors as JSON
* Convert controller specs to request specs to catch OpenAPI problems
* Switch from sqlite to postgres for the underlying database
* Spin up postgres in CI
* Add .dockerignore file to keep images svelte
* Update Dockerfile
* Update README
* Re-generate binstubs and add puma binstub
* Add docker-compose configuration with web and db services
@mjgiarlo mjgiarlo changed the title [HOLD] Update docker infra and fix image building and publishing in CI Update docker infra and fix image building and publishing in CI Apr 9, 2020
parameters:
- name: id
in: path
description: druid to return
required: true
schema:
$ref: '#/components/schemas/BareDruid'
/suri2/namespaces/druids/identifiers:
/suri2/namespaces/druid/identifiers:
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 was what was causing the Hydrus build to fail, @ndushay, if you were interested. I added a bunch of request specs to this branch to catch these more easily. (This is part of why we've been moving away from controller specs towards request specs!)

@mjgiarlo mjgiarlo merged commit 497403c into master Apr 9, 2020
@mjgiarlo mjgiarlo deleted the fix-image-build branch April 9, 2020 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants