-
-
Notifications
You must be signed in to change notification settings - Fork 10
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: use dynamic ports for router and mailpit, fixes #23 #30
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @zeshanziya !
Yes, that's the right approach, and while we're here, let's update the Mailpit ports too.
DDEV template for the web
container has the same env variables https://github.com/ddev/ddev/blob/db18c34739faecd597fbdbb36c6ad743a82e8c45/pkg/ddevapp/app_compose_template.yaml#L203-L208
This PR would be massively better if it had a test for this situation... |
A new test should be added to Basically, it should be a copy-paste "install from directory" with the addition of:
And the check should be done for other ports in the test itself instead of the standard ones. |
Co-authored-by: Stanislav Zhuk <[email protected]>
@stasadev Thank you for the guidance. I have updated the Mailpit ports as suggested and added a test case for non-standard ports. Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great and the new test works, thank you!
The Issue
Issue #23
How This PR Solves The Issue
Updated the docker-compose.varnish.yaml file to use environment variables (
DDEV_ROUTER_HTTPS_PORT
andDDEV_ROUTER_HTTP_PORT
) for dynamic port configuration.This change ensures compatibility with custom DDEV port settings and resolves issues when default ports are unavailable
Manual Testing Instructions
And look inside:
Automated Testing Overview
Related Issue Link(s)
Release/Deployment Notes