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

fix: add swapfile #232

Merged
merged 1 commit into from
May 11, 2023
Merged

Conversation

MichaelDeBoey
Copy link
Member

@MichaelDeBoey MichaelDeBoey merged commit c312b37 into remix-run:main May 11, 2023
6 checks passed
@MichaelDeBoey MichaelDeBoey deleted the add-swapfile branch May 11, 2023 00:55
@mikeybinns
Copy link

I understand the need for the swapfile, but can this be implemented using the swap_size_mb fly.toml option as a cleaner solution than requiring a shell script?
Documented here:
https://fly.io/docs/reference/configuration/#swap_size_mb-option

@MichaelDeBoey
Copy link
Member Author

@rubys if that's the case, I guess we can simplify dockerfile-node as well?

@rubys
Copy link

rubys commented Aug 23, 2023

@MichaelDeBoey yes that is correct. And dockerfile-node already has support for this: fly-apps/dockerfile-node@d489e3a ... and if you check the test cases you will see that this already is enabled and tested for indie and blues stacks.

Any chance remix could consider switching to dockerfile node?

@MichaelDeBoey
Copy link
Member Author

@rubys if we can support everything that's happening right now in the Dockerfile, I'd be happy to accept a PR that changes docs and CI & removes the current version of Dockerfile, fly.toml, ... in both indie-stack & blues-stack

@rubys
Copy link

rubys commented Aug 23, 2023

I believe we can support everything. If we discover something that needs to be added that can be done quickly, so I'm not worried about that.

The doc changes should be pretty straightforward. Much of the current README can be replaced by fly launch.

The CI will likely take several iterations to get right. I've already got a test that builds the Dockerfile. Example: https://github.com/fly-apps/dockerfile-node/actions/runs/5933428308/job/16088842745

If looks like your existing test deploys the app on fly.io. It doesn't create the application, so it presumes that the app is already present?

So the next step is presumably to figure out what test is needed, subtract out what is already covered, and then write a test for what remains. Whether that test or tests resides in a remix repository or a fly apps repository doesn't matter to me.

@MichaelDeBoey
Copy link
Member Author

@rubys Let's continue the discussion in #252

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.

Fly Instance runs out of memory on deployment
4 participants