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

add admin option to php-server for easier debugging #932

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

withinboredom
Copy link
Collaborator

This just adds a --admin option for the php-server command. Makes it a heck of a lot easier to profile and debug.

@withinboredom
Copy link
Collaborator Author

@dunglas any objections to merging this?

@dunglas
Copy link
Owner

dunglas commented Aug 14, 2024

I'm hesitating. I would prefer keeping the "general" Caddy-related options in sync with what native Caddy commands provide.

What do you think @mholt @francislavoie?

@francislavoie
Copy link
Contributor

francislavoie commented Aug 14, 2024

Why not just use a Caddyfile?

I don't really have an opinion on the design of a command that isn't included in vanilla Caddy.

@withinboredom
Copy link
Collaborator Author

Personally, I'm not a fan of it either except for exactly one case: quickly switching between test directories without maintaining a caddy file and all their variants. Being able to turn on admin and running pprof is great.

That being said, it is also too easy to just go into the file and change it to true. So, I'm not particularly attached to it one way or the other. I didn't think about being able to use the admin api to change settings, so I'm leaning towards closing the PR.

@mholt
Copy link
Contributor

mholt commented Aug 14, 2024

Similar to Francis, if it's not Caddy itself I don't mind if other programs add their own CLI. But if you're going for parity with the caddy command (there is wisdom in that, to keep it more predictable and symmetric), I would just flip the switch in a config file, personally.

Caddy values having all the config contained in a single file/source, not spread across config file and CLI and env vars etc ... but it's really up to the application developer.

@francislavoie
Copy link
Contributor

@mholt in case you didn't realize, this command is more akin to caddy file-server or caddy reverse-proxy kinda thing

@mholt
Copy link
Contributor

mholt commented Aug 14, 2024

That's true. In that case it doesn't really matter then IMO. Since those specialty sub commands carry config on them.

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.

4 participants