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

Apply CGI environment variables in the correct order #3718

Merged
merged 5 commits into from
Mar 29, 2023

Conversation

syrusakbary
Copy link
Member

This PR does the following:

  • Allows receiving a --debug/-d flag to the run-unstable subcommand
  • Removes the SCRIPT_NAME hack (it should be done by the package annotation)
  • Prints the server where things are running (I got so lost before that I had no idea in which port it was running)

@Michael-F-Bryan
Copy link
Contributor

I've updated this PR to remove the --debug flag. We actually enable the logger already at the start of the function - you can increase the global log level by adding more -v arguments (default is warn) and can also use $RUST_LOG for fine-tuning.

I'd prefer not to pass Config to the callbacks if we can. For a situation like this, the caller already knows the address we're serving from because they set up the runner, so we can just reuse it.

I also reorganised the order we set environment variables in so we'll apply the CGI variables first and give users the ability to override them. I felt that was better than https://github.com/wasmerio/wcgi/pull/37 because it means you still get a useful default when you don't $DOCUMENT_ROOT and $SCRIPT_FILENAME, rather than erroring out because mod_cgi.wasm doesn't know which script to invoke.

@Michael-F-Bryan Michael-F-Bryan changed the title Allow to receive custom configuration Apply CGI environment variables in the correct order Mar 29, 2023
Copy link
Contributor

@theduke theduke left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Michael-F-Bryan Michael-F-Bryan merged commit dc977cd into master Mar 29, 2023
@Michael-F-Bryan Michael-F-Bryan deleted the wcgi-fixes branch March 29, 2023 18:01
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.

3 participants