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

Add flightcontrol deploy command #4788

Merged
merged 7 commits into from
Mar 19, 2022

Conversation

beerose
Copy link
Contributor

@beerose beerose commented Mar 17, 2022

This PR adds rw setup deploy flightcontrol and rw deploy flightcontrol commands.

Link to the docs PR: #4788


Had some problems with testing the rw deploy command. I have a minimal redwood project, and running yarn dev deploy flightcontrol web --cwd <path to my project> was resulting in Could not find a "redwood.toml" file, are you sure you're in a Redwood project? error. The file was there and it looked like the command was run in the correct directory. Any pointers would be appreciated 🙏

@netlify
Copy link

netlify bot commented Mar 17, 2022

✅ Deploy Preview for redwoodjs-docs canceled.

🔨 Explore the source changes: bcd2a71

🔍 Inspect the deploy log: https://app.netlify.com/sites/redwoodjs-docs/deploys/62357d761e9e28000958bf99

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@cannikin
Copy link
Member

cannikin commented Mar 17, 2022

Hello! Welcome and thanks for the PR! We've been big fans of @flybayer since we both started our frameworks around the same time. We're rooting for him and the team with Flightcontrol!

That command you pasted, yarn dev deploy flightcontrol web should really be yarn rw deploy flightcontrol web (rw instead of dev). Maybe that's the issue?

Whoops! Turns out yarn dev is a real command that I didn't even know existed! 😬 Ignore me, see @thedavidprice's comment below!

@thedavidprice thedavidprice self-assigned this Mar 17, 2022
@thedavidprice thedavidprice self-requested a review March 17, 2022 17:03
@dac09 dac09 added the release:feature This PR introduces a new feature label Mar 17, 2022
@thedavidprice thedavidprice marked this pull request as draft March 17, 2022 17:13
@thedavidprice
Copy link
Contributor

@jtoar FYI re:

yarn dev deploy flightcontrol web --cwd

^^ I've noticed this isn't working for me locally as well. My guess is that it has something to do with recent move to Yarn 3

@thedavidprice
Copy link
Contributor

@beerose This is amazing! Excited to help you get this into Redwood.

Local Dev
Gah, I think we busted that --cwd command during our recent migration to Yarn 3. (Also, I'm assuming you have a local test project and are setting --cwd at that project outside the Framework codebase, correct?)

What you can use instead is the rwfw project:sync command, which you run from the local test project. Here's the doc explaining what that is and how it works:

Also, here are the two main Contributing docs for ongoing reference:

Deploy Target CI Repo
I've followed up with you directly about next steps to first set this up over here: https://github.com/redwoodjs/deploy-target-ci

That's the best way I can understand how everything works in order to review this PR and make sure all the pieces are connected!

if (serve) {
console.log('\nStarting api...')
await apiServerHandler({
port: getConfig().api?.port || 8911,
Copy link
Contributor

Choose a reason for hiding this comment

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

I updated this because it was broken as is. Assuming you'll be port forwarding. Otherwise, I recommend updating in redwood.toml or just hardcoding here.

Comment on lines +145 to +146
// We need to set the apiUrl evn var for local dev
const addToDotEnvDefaultTask = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Because the apiUrl was changed to use an env var, we need to make sure there's a value for local dev

Comment on lines +17 to +18
buildCommand: 'yarn rw deploy flightcontrol api',
startCommand: 'yarn rw deploy flightcontrol api --serve',
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI updated these commands

name: 'Redwood Web',
type: 'static',
singlePageApp: true,
buildCommand: 'yarn rw deploy flightcontrol web',
Copy link
Contributor

Choose a reason for hiding this comment

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

This one as well

Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

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

@beerose @flybayer Thanks again for getting this going! I've fixed some issues and made some config and command changed -- do review my changes, please, to confirm they'll all work with Flighcontrol.

So we can move forward, I'm going to merge this and use the corresponding published canary version over here: https://github.com/redwoodjs/deploy-target-ci

Things aren't quite working, so we'll keep at it!

@thedavidprice thedavidprice marked this pull request as ready for review March 19, 2022 06:49
@thedavidprice thedavidprice merged commit 2d25ecb into redwoodjs:main Mar 19, 2022
@jtoar jtoar added this to the next-release milestone Mar 19, 2022
@beerose beerose deleted the flightcontrol-deploy branch March 22, 2022 12:52
@thedavidprice thedavidprice modified the milestones: next-release, v0.50.0 Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
No open projects
Status: Archived
Development

Successfully merging this pull request may close these issues.

None yet

5 participants