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

Updates Dockerfile to prepare for production #66

Merged
merged 54 commits into from
May 10, 2024
Merged

Conversation

bryanlopezrr
Copy link
Contributor

Cleaning up Dockerfile, setting up local_setup script (for local development), modifying docker-compose to include environment variables (these environment variables will need to be configured in the build environment as well in circle ci)

@bryanlopezrr bryanlopezrr requested a review from a team February 16, 2024 21:23
@bryanlopezrr
Copy link
Contributor Author

I didnt have access to the circle ci, pipeline before so I have to make sure that it passes before moving forward

@whereismyjetpack whereismyjetpack changed the base branch from main to k8s February 16, 2024 21:42
@whereismyjetpack whereismyjetpack changed the base branch from k8s to main February 16, 2024 21:43
Copy link
Contributor

@whereismyjetpack whereismyjetpack left a comment

Choose a reason for hiding this comment

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

Thanks for this!

Once these changes are addressed, I'll be able to test locally and provide final feedback. Some of these suggestions are to be more in line with our other apps, others are best practices to reduce layers/size in the resulting image.

Please ask any questions you might have

.sample.env Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
local_setup.sh Outdated Show resolved Hide resolved
Gemfile Outdated Show resolved Hide resolved
config/credentials.yml.enc Outdated Show resolved Hide resolved
config/environments/production.rb Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Gemfile Outdated Show resolved Hide resolved
@whereismyjetpack whereismyjetpack changed the title Appint 192 Updates Dockerfile to prepare for production Feb 21, 2024
@bryanlopezrr
Copy link
Contributor Author

@whereismyjetpack should we re-review this and merge? It fell through the cracks on my to-do items. I can't remember if we are good to merge or what the next steps will be

@whereismyjetpack
Copy link
Contributor

we'll need @ajkiessl to test locally to make sure it works for him

Copy link
Contributor

@ajkiessl ajkiessl left a comment

Choose a reason for hiding this comment

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

Most of this looks good. I just added one change request for the db seeding. Everything seems to be working when I run this locally.

The QA deployment has some issues, though. First, the db seed wasn't there so I manually ran that to get the integrations page working. Second, there are some configurations that aren't being loaded. We'll have to come up with a strategy for these. The two configs needed are activity_insight.yml and the integration_passcode.yml. The .circleci versions of these in source control map out what values we need. I'm assuming we'll want to use the same strategy the database.yml is using (loading ENV variables). I can meet to go over these if you want. Finally, there's a directory that needs added to store the post print files. I'm getting this error when I go to that page: Errno::ENOENT (No such file or directory @ dir_initialize - /app/public/post_prints). So, I think that directory just needs added.

I'm assuming the files and database data will be migrated from the old setup to the new setup when we push this to production?

bin/startup Show resolved Hide resolved
Copy link
Contributor

@ajkiessl ajkiessl left a comment

Choose a reason for hiding this comment

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

I'd say this is good to go

@whereismyjetpack whereismyjetpack merged commit f20c16d into main May 10, 2024
1 check passed
@ajkiessl ajkiessl deleted the APPINT-192 branch May 10, 2024 17:41
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.

4 participants