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

Provisioning Fixes #1387

Merged
merged 4 commits into from
Nov 5, 2024
Merged

Provisioning Fixes #1387

merged 4 commits into from
Nov 5, 2024

Conversation

rajadain
Copy link
Collaborator

@rajadain rajadain commented Oct 18, 2024

Overview

Updates to provisioning to get this project to setup. With virtualbox now being the default for this app, we would no longer need to specify CAC_APP_SHARED_FOLDER_TYPE=virtualbox before running vagrant commands on Windows and Linux.

Notes

By tying our NPM version to Ubuntu we make it less explicit, and potentially risk having it change when we do an Ubuntu upgrade. However, it was the easiest way to get a reliably working environment, and since the associated Ansible Role is old and unmaintained, I think it's alright.

Testing Instructions

  • Check out this branch
  • Remove your local node_modules folder
    rm -rf src/node_modules
  • Delete and reprovision the app VM
    vagrant destroy -f app && vagrant up app
    • Ensure it provisions correctly
  • Build the front-end
    ./scripts/serve-js-dev.sh
  • Go to http://localhost:8024/
    • Ensure you see the running app

Checklist

  • No gulp lint warnings
  • No python lint warnings
  • Python tests pass
  • All TODOs have an accompanying issue link

Connects #XXX

Copy link
Collaborator

@rachelekm rachelekm left a comment

Choose a reason for hiding this comment

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

Thank you for these fixes! I'm able to successfully provision and see the running app, but I'm having trouble accessing the django admin at localhost:8024/admin. I get a 301 status code and it will redirect to localhost:443/admin are you seeing something similar?

@rajadain
Copy link
Collaborator Author

I'm not exactly sure if this is the true explanation, but I believe this block:

server {
listen 80;
server_name gophillygo.org www.gophillygo.org;
return 301 https://$host$request_uri;
}

Redirects http://localhost:8024/admin (which doesn't match any existing nginx configuration) to http://localhost:443/admin (via the return 301 line), which then fails because the front-end then goes to :443 rather than :8024 as the Vagrantfile mapping expects:

cac-tripplanner/Vagrantfile

Lines 138 to 139 in 71a8966

# Web
app.vm.network "forwarded_port", guest: 443, host: 8024

By making the trailing slash optional, we now allow http://localhost:8024/admin to behave identically to http://localhost:8024/admin/, both of which now correctly redirect.

Fixed in bd5b3d4.

Will require reprovisioning app. Ready for another look.

@rachelekm rachelekm self-requested a review October 29, 2024 18:24
Copy link
Collaborator

@rachelekm rachelekm left a comment

Choose a reason for hiding this comment

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

Thank you, @rajadain! I re-provisioned and the app as well as the admin page work as expected 👍 Thanks for this work, those fixes will really improve any future spin-up and dev experience!

NFS makes sense for Windows developers, but since most
of our team is on Mac and Linux, having virtualbox be
the default is better.
Previously we were explicitly installing NodeJS 12 and NPM 6,
which caused various provisioning issues. By switching to
NodeJS 12 and NPM 8 which come native with this version of
Ubuntu, we ensure that provisioning succeeds.
This is done automatically when using NPM 8.
@rajadain rajadain force-pushed the tt/provisioning-fixes branch from bd5b3d4 to bb5e055 Compare October 29, 2024 18:37
@rajadain
Copy link
Collaborator Author

Before I merge this, I tried out the tests, and this one is failing:

def test_isochrone_partitioning(self):

Can you check on your end? It's possible I'm missing something in my local setup.

@rachelekm
Copy link
Collaborator

test_isochrone_partitioning

Strange! Luckily the tests are successful for me:

----------------------------------------------------------------------
Ran 36 tests in 4.424s

OK

I did some digging and looks like Katie bumped into this same issue in the past and it could be a machine-dependent failure.

@rajadain rajadain merged commit 256bb73 into develop Nov 5, 2024
@rajadain rajadain deleted the tt/provisioning-fixes branch November 5, 2024 16:21
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