Skip to content

SL-667: Add CICD config for Javabuilder demo env #385

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

Merged
merged 4 commits into from
Apr 11, 2023

Conversation

ebeastlake
Copy link
Contributor

@ebeastlake ebeastlake commented Mar 31, 2023

Adds a new javabuilder-demo stack for Java Lab eval mode.

Tickets:

Outstanding questions:

@ebeastlake
Copy link
Contributor Author

@molly-moen I still need to add something here, yeah? https://github.com/code-dot-org/javabuilder/blob/main/cicd/3-app/javabuilder/template.yml.erb#L1118

I specified the javabuilder-demo subdomain in the YML file config, so this template should produce wss://javabuilder-demo.code.org, but where does that value get exposed? (i.e. These templates and URLs are 1-1 with the env files, but I need dashboard to be 1-2 with URLs, and I don't see where I do something like JavabuilderDemoURL = *).

"LimitPerHour": "25",
"LimitPerDay": "100",
"SilenceAlerts": "false",
"TeacherLimitPerHour": "5000"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is what you're discussing in other comments. For clarity, I'd suggest adding this to the other config files with whatever our default value is.

Copy link
Contributor

Choose a reason for hiding this comment

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

But this would be safe to deploy as-is. You could also remove that config parameter at this time, and add it later as it's own PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

In fact, that's what I'd recommend. Remove the TeacherLimitPerHour part from this PR, then we can merge this and make sure we've got a green pipeline into this new environment before adding new config parameters.

Copy link
Contributor

@cat5inthecradle cat5inthecradle left a comment

Choose a reason for hiding this comment

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

LGTM! To deploy this change, after merging, we just need to run the following.

cicd/2-cicd/deploy-cicd.sh

There might be a bit of pain to do with ruby versions or AWS credentials. Once you merge, switch back to main on your local, pull this new code, then run cicd/2-cicd/deploy-cicd.sh. Ping me on slack and I can follow along and help troubleshoot any issues.

@sanchitmalhotra126
Copy link
Contributor

sanchitmalhotra126 commented Apr 10, 2023

I specified the javabuilder-demo subdomain in the YML file config, so this template should produce wss://javabuilder-demo.code.org, but where does that value get exposed? (i.e. These templates and URLs are 1-1 with the env files, but I need dashboard to be 1-2 with URLs, and I don't see where I do something like JavabuilderDemoURL = *).

Yep, updating Dashboard (and the frontend) would be a separate change -

Dashboard reads the Javabuilder websocket and HTTP endpoints here and here (in cdo.rb). Currently we just default to the prod URL (unless overridden via DCDO, or by local environment config), so we'll have to update some logic to serve the correct URL to the client based on the situation. We read the websocket URL value from the config in our back end here in javalab.rb, which is served to the front end as part of the level info. The actual websocket request, using the given URL, is made here (in JavabuilderConnection). The HTTP url is used in the backend only when making the initial put sources call; that's read here (in javalab_files_helper).

Copy link
Contributor

@sanchitmalhotra126 sanchitmalhotra126 left a comment

Choose a reason for hiding this comment

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

One question out of curiosity, but otherwise LGTM!

Configuration:
StackName:
!If [
TargetsMainBranch,
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a question for @cat5inthecradle - is it possible/should it be possible to target a branch other than main for a production deploy?

@cat5inthecradle cat5inthecradle merged commit 738085a into main Apr 11, 2023
@cat5inthecradle cat5inthecradle deleted the task/sl-667/javabuilder-demo-ci-cd branch April 11, 2023 19:11
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.

3 participants