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

feat: PHP SPX #820

Merged
merged 3 commits into from
Nov 19, 2024
Merged

feat: PHP SPX #820

merged 3 commits into from
Nov 19, 2024

Conversation

SamJUK
Copy link
Contributor

@SamJUK SamJUK commented Nov 17, 2024

Supporting work to make use of the new PHP SPX images introduced in wardenenv/images#17

PR for Supporting documentation: wardenenv/docs#25

Key Points:

  • New boolean feature flag WARDEN_PHP_SPX to toggle feature state
  • Feature flag added to each .env init template, with a default of disabled
  • Update warden shell to automatically pick the correct container depending on the feature flag

@@ -7,7 +7,9 @@ loadEnvConfig "${WARDEN_ENV_PATH}" || exit $?
## set defaults for this command which can be overridden either using exports in the user
## profile or setting them in the .env configuration on a per-project basis
WARDEN_ENV_SHELL_COMMAND=${WARDEN_ENV_SHELL_COMMAND:-bash}
WARDEN_ENV_SHELL_CONTAINER=${WARDEN_ENV_SHELL_CONTAINER:-php-fpm}

WARDEN_ENV_DEFAULT_SHELL_CONTAINER=$([ $WARDEN_PHP_SPX == 1 ] && echo "php-spx" || echo "php-fpm")
Copy link
Member

Choose a reason for hiding this comment

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

@SamJUK I don't fully agree with this. The warden shell goes to the standard php-fpm container and warden debug goes to php-debug container. To me this would seem like we could either add a flag --spx or create a new warden spx command to drop to a shell in it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also prefer having warden spx command, similar to warden debug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I'll switch it over to a separate command warden spx (since AFAIK the flag approach is not widely used across the project)

@navarr navarr added the enhancement New feature or request label Nov 18, 2024
@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Nov 19, 2024

Great job @SamJUK!
It's great to see how the feature that I suggested in https://github.com/orgs/wardenenv/discussions/719 evolved to a real built-in feature of warden!
Waiting for these changes to be merged and delivered as a release that we can use on daily basis :)

@navarr navarr added this pull request to the merge queue Nov 19, 2024
Merged via the queue into wardenenv:main with commit 3090dd1 Nov 19, 2024
@monteshot
Copy link
Contributor

I updated Warden (alternative installation)
Added to project file(.env):

WARDEN_PHP_SPX=1

Then ran
warden svc pull; warden svc up; warden env pull; warden env up;

I successfully get SPX UI
image.

Then I disabled SPX

WARDEN_PHP_SPX=0

And run

warden env down; warden env up;

The Project is not working anymore
image
image

@SamJUK
Copy link
Contributor Author

SamJUK commented Nov 21, 2024

@monteshot Can you make sure you do not have any SPX related cookies remaining? Or try in Incognito / with another browser?

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Nov 21, 2024

@monteshot,
I think the easiest option to fix that would be:

Another option - remove SPX_* cookies from your browser

I guess it has to be added to documentation wardenenv/docs#25 (comment)

@SamJUK
Copy link
Contributor Author

SamJUK commented Nov 21, 2024

Opened a separate issue to track the 502 bad gateway #822

It would be nice if we can avoid adding manual steps to not break your environment when disabling SPX, I can see that causing frustration otherwise.

@monteshot
Copy link
Contributor

I tried to Disable SPX from the UI... I think there is an issue because SPX cookies are still present.

"Another option - remove SPX_* cookies from your browser"

This solution works as a workaround. I think an additional mechanism(on Traefik or somewhere else) should be created only for disabling the feature.

@bap14
Copy link
Member

bap14 commented Nov 21, 2024

@SamJUK Unfortunately there are going to end up being manual steps no matter what because disabling the service won't update the cookies in your browser. Just like if you stopped the xdebug container and then sent a request with the XDEBUG_SESSION cookie set it would also result in an error. To my knowledge there isn't a PHP-SPX chrome extension, but that could be a potential solution for this, but that's also technically a manual process as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants