Skip to content

Make test the default branch #108

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 12 commits into from
Jun 16, 2025
Merged

Make test the default branch #108

merged 12 commits into from
Jun 16, 2025

Conversation

eberrigan
Copy link
Collaborator

No description provided.

7174Andy and others added 12 commits June 12, 2025 12:30
* implement scp

* fix name

* fix var name

* fix type of dir

* add scp to image

* change to raw for private key

* copy from docker container

* solve permission for docker

* skip if .slp does not exist

* fix temp file dne

* fix file structure

* auth for downloading analytics

* changed mechanism for zip

* remove slp files in model and predictions dir

* fix according to comments

* Change to port of the Allocator Web app from 5000 to 80 (#73)

* change to port 80

* fix to accept port 5000

* expose 5000

* block port 5000

* test

* change to prod

* add button for extract

* implement scp

* fix name

* fix var name

* fix type of dir

* add scp to image

* change to raw for private key

* copy from docker container

* solve permission for docker

* skip if .slp does not exist

* fix temp file dne

* fix file structure

* auth for downloading analytics

* changed mechanism for zip

* remove slp files in model and predictions dir

* fix according to comments

* add button for extract

* handle no vm case

* modify design for deletion page

* enable download

* redesign

* preserve directory

* fix bug

* change to rsync

* fix options

* fix error final

* fix error for rsync

* preserve path when zipping

* doc

* resolve comments

* cleanup implemented after request

* change button name
* add new config for ami

* fix terraform var

* change AMI for allocator

* add new ami for client vm

* change AMI

* change resource

* change AMI
* init

* change workflow for client service

* fix correct function

* always display message

* fix success message

* fix json format

* resolve comments

* Make Client VM's AMI ID Configurable (#90)

* add new config for ami

* fix terraform var

* change AMI for allocator

* add new ami for client vm

* change AMI

* change resource

* change AMI

* init

* change workflow for client service

* fix correct function

* always display message

* fix success message

* fix json format

* resolve comments
This reverts commit 1a60a92.
* use eip

* add resource suffix variable for testing

* remove erroneous code

* update devcontainer config

* make backend configurable

* prod backend

* test backend

* make backend configurable

* configure action per environment

* remove old backend

* extract ssh key from terraform output

* test local backend

* get allocator ip and key pair from terraform

* configure keyname using env

* output vm info

* generate private key in terraform

* pass in eip and key name

* add allocator ip and key name as env variables

* debug statement

* add route53

* manage domain and route53 in aws console for permanent eip

* output subdomain record

* manual deployment for all branches

* save pem key as artifact

* backend s3 for prod and test

* duplicate aws key pair error and cleanup

* add tutorial data

* Revert "duplicate aws key pair error and cleanup"

This reverts commit 9e005a6.

* Revert "Merge branch 'main' into elizabeth/route-ip-to-domain"

This reverts commit 76989c8, reversing
changes made to 55a9f75.

* pass in environment

* normalize just in case

* resource suffix is passed in

* make security group name more readable

* prepare for rebase

* resolve conflicts

* Remove import

---------

Co-authored-by: 7174Andy <[email protected]>
* back to admin when credential successful:

* for user to type aws credentials if missing

* back to admin when credential successful:

* for user to type aws credentials if missing

* Route IP address to domain (#75)

* use eip

* add resource suffix variable for testing

* remove erroneous code

* update devcontainer config

* make backend configurable

* prod backend

* test backend

* make backend configurable

* configure action per environment

* remove old backend

* extract ssh key from terraform output

* test local backend

* get allocator ip and key pair from terraform

* configure keyname using env

* output vm info

* generate private key in terraform

* pass in eip and key name

* add allocator ip and key name as env variables

* debug statement

* add route53

* manage domain and route53 in aws console for permanent eip

* output subdomain record

* manual deployment for all branches

* save pem key as artifact

* backend s3 for prod and test

* duplicate aws key pair error and cleanup

* add tutorial data

* Revert "duplicate aws key pair error and cleanup"

This reverts commit 9e005a6.

* Revert "Merge branch 'main' into elizabeth/route-ip-to-domain"

This reverts commit 76989c8, reversing
changes made to 55a9f75.

* pass in environment

* normalize just in case

* resource suffix is passed in

* make security group name more readable

* prepare for rebase

* resolve conflicts

* Remove import

---------

Co-authored-by: 7174Andy <[email protected]>

* change to v4

* back to admin when credential successful:

* for user to type aws credentials if missing

---------

Co-authored-by: Elizabeth <[email protected]>
* resolve issues with scp

* resolve the launch api

* Revert "resolve the launch api"

This reverts commit 91c05c0.

* fix launch api

* remove error code:

* fix error in scp

* remove unused function

* add key name debug:
* prod image triggers terraform prod

* prod image used in terraform deployment for prod env

* cleanup workflow

* allocator image tag passed to terraform for allocator deployment

* make destroy env dependent

* determine image tag from env

* add latest test as default
@eberrigan eberrigan requested a review from Copilot June 16, 2025 22:23
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors environment handling by making test the default branch for Terraform workflows and adds UI/API improvements for VM management.

  • CI/CD: Update GitHub Actions to target test and main branches and support dynamic environment selection with per-env backend config.
  • UI/API: Add real-time unassigned VM count polling, .slp extraction/download UI and endpoints, and AWS credential error alerts.
  • Config/schema: Introduce ami_id and repository in structured config, and provide environment-specific Terraform backend HCL files.

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
templates/index.html Added live unassigned VM count display with polling script.
templates/delete-instances.html Revamped delete page UI, added extract form, spinner, success/error.
templates/dashboard.html Introduced AWS credentials error alert block.
templates/admin.html Updated button text and added AWS-credentials success message.
main.py Added /api/scp-client and /api/unassigned_vms_count endpoints; ANSI-escape stripping; enhanced error handling.
database.py Added get_row_count method.
conf/structured_config.py Added new ami_id field to MachineConfig.
conf/config.yaml Updated ami_id and repository defaults.
backend_config.tf, backend.tf, backend-*.hcl Removed static backend; added per-env HCL for dynamic backend.
Dockerfile Installed rsync and openssh-client for file transfers.
.devcontainer/devcontainer.json Removed unnecessary features; adjusted forwarded ports.
.github/workflows/*.yml Constrained workflows to test/main; added environment/image input logic.
Comments suppressed due to low confidence (4)

lablink-allocator/lablink-allocator-service/main.py:352

  • [nitpick] Consider adding a docstring above this route to describe its purpose, input requirements, and response format for better maintainability.
@app.route("/api/scp-client", methods=["GET"])

lablink-allocator/lablink-allocator-service/main.py:432

  • [nitpick] This new endpoint lacks unit or integration tests. Consider adding tests to verify the JSON response under normal and empty-database conditions.
@app.route("/api/unassigned_vms_count", methods=["GET"])

lablink-allocator/lablink-allocator-service/main.py:166

  • [nitpick] This /api/admin/set-aws-credentials route returns HTML instead of JSON. For consistency, either rename the endpoint to remove /api or return a JSON payload and handle the success message client-side.
return render_template("admin.html", message="AWS credentials set successfully.")

lablink-allocator/lablink-allocator-service/main.py:51

  • The code references os.getenv and later os.remove without importing the os module. Please add import os at the top of the file to avoid a NameError.
allocator_ip = os.getenv("ALLOCATOR_PUBLIC_IP")

Comment on lines +105 to +114
window.location.href = "/api/scp-client";

setTimeout(() => {
loading.classList.add("d-none");
btn.disabled = false;
btn.innerText = "Extract and Download .slp Files";
btn.classList.remove("disabled");
destroyBtn.disabled = false;
destroyBtn.classList.remove("disabled");
}, 10000);
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

[nitpick] Navigating via window.location.href unloads the page immediately, preventing the spinner and cleanup logic from running. Consider using fetch or XMLHttpRequest to download the file and manage the UI state.

Suggested change
window.location.href = "/api/scp-client";
setTimeout(() => {
loading.classList.add("d-none");
btn.disabled = false;
btn.innerText = "Extract and Download .slp Files";
btn.classList.remove("disabled");
destroyBtn.disabled = false;
destroyBtn.classList.remove("disabled");
}, 10000);
try {
const response = await fetch("/api/scp-client");
if (!response.ok) {
throw new Error("Failed to download files");
}
const blob = await response.blob();
const url = URL.createObjectURL(blob);
const a = document.createElement("a");
a.href = url;
a.download = "files.slp";
document.body.appendChild(a);
a.click();
URL.revokeObjectURL(url);
success.classList.remove("d-none");
} catch (err) {
error.classList.remove("d-none");
console.error("Error during extraction:", err);
} finally {
loading.classList.add("d-none");
btn.disabled = false;
btn.innerText = "Extract and Download .slp Files";
btn.classList.remove("disabled");
destroyBtn.disabled = false;
destroyBtn.classList.remove("disabled");
}

Copilot uses AI. Check for mistakes.

@eberrigan eberrigan merged commit 3682031 into test Jun 16, 2025
4 of 6 checks passed
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