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

Build arm containers with new github actions arm runner (PP-2139) #765

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

jonathangreen
Copy link
Member

@jonathangreen jonathangreen commented Feb 11, 2025

Description

This PR depends on #766 to update pre-commit hooks. This is merged.

It is similar to ThePalaceProject/circulation#2280 and follows the same pattern.

This refactors our docker build for the library registry to use a native arm runner to build the arm version of the library registry. It follows the pattern that the circulation repo is using, moving the docker build out to its own workflow, rather than having it as part of the test workflow.

NOTE: 🚨 Right now there isn't a great way to run our tests in the docker containers before pushing them. I put a todo in for this, and there is a ticket in JIRA here PP-2142. The current workflow will unconditionally push the containers. This shouldn't be a big deal, everything merged to main should be tested before merging, so there aren't many cases where tests in container should fail. I'd like to add this in eventually, but I think it can be a follow up.

Motivation and Context

See PP-2139. This will fix broken docker builds like the one in this PR #764

How Has This Been Tested?

  • Running in CI

Checklist

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

@jonathangreen jonathangreen force-pushed the chore/workflow-arm-runner branch from 4de546e to d59c20c Compare February 11, 2025 15:10
@jonathangreen jonathangreen changed the base branch from main to chore/update-precommit February 11, 2025 15:10

CREATE DATABASE simplified_registry_test;
CREATE USER simplified_test WITH PASSWORD 'simplified_test';
GRANT ALL PRIVILEGES ON DATABASE simplified_registry_test TO simplified_test;
ALTER DATABASE simplified_registry_test OWNER TO simplified_test;
Copy link
Member Author

Choose a reason for hiding this comment

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

These changes are due to the PG16 upgrade that we did, and could move out to a separate PR if desired.

Similar to changes for PG16 needed here: https://github.com/ThePalaceProject/hosting-playbook/pull/720

@jonathangreen jonathangreen requested a review from a team February 11, 2025 15:22
@jonathangreen jonathangreen marked this pull request as draft February 11, 2025 15:29
Copy link
Collaborator

@tdilauro tdilauro left a comment

Choose a reason for hiding this comment

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

Looks good! 🚀

Base automatically changed from chore/update-precommit to main February 11, 2025 16:39
@jonathangreen jonathangreen force-pushed the chore/workflow-arm-runner branch from d59c20c to 7f308e9 Compare February 11, 2025 16:41
@jonathangreen jonathangreen marked this pull request as ready for review February 11, 2025 16:43
@jonathangreen jonathangreen merged commit 0d52f79 into main Feb 11, 2025
8 checks passed
@jonathangreen jonathangreen deleted the chore/workflow-arm-runner branch February 11, 2025 16:45
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