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 bypass for WP-CLI in Pantheon_Sessions->load() #150

Closed
wants to merge 1 commit into from

Conversation

mikeschinkel
Copy link

This to eliminate Cannot change save handler when headers already sent being written to the error log.

…ot change save handler when headers already sent` being written to the error log.
@danielbachhuber
Copy link
Contributor

Thanks @mikeschinkel ! Landing this in #151 because of build status limbo:

image

@danielbachhuber
Copy link
Contributor

@mikeschinkel Actually, there was a reason WP Native PHP Sessions runs during WP-CLI #91

If you'd like to disable it on your own, here's documentation on how to do so: #80 (comment)

@mikeschinkel
Copy link
Author

mikeschinkel commented May 11, 2020

@danielbachhuber — I see. Thanks for the follow up.

Suggestion? Rather than enabling it to fix the problem, what about creating no-op mocks for each of the functions and have them loaded when WP-CLI is in use?

I can of course use the workaround to meet my current needs. Thanks.

@danielbachhuber
Copy link
Contributor

Suggestion? Rather than enabling it to fix the problem, what about creating no-op mocks for each of the functions and have them loaded when WP-CLI is in use?

That could potentially work. Happy to look at a pull request if you'd like to put one together.

@mikeschinkel
Copy link
Author

Happy to look at a pull request if you'd like to put one together

@danielbachhuber — Thanks for considering. I am looking into the potential of this right now.

@mikeschinkel
Copy link
Author

@danielbachhuber — So I started down the path of trying to mock and it quickly seemed like it would be a maintenance nightmare for you.

So instead I went another direction, and this the fix to stop having it pollute the logs ends up being very simple; set session.use_cookies to '0' when WP CLI is running. Only the following change is required on line 144 of pantheon-sessions.php:

ini_set( 'session.use_cookies', defined( 'WP_CLI' ) && WP_CLI ? '0' : '1' );

If you agree I can either create a pull request, or given how simple it is maybe you can just apply it?


BTW, setting PANTHEON_SESSIONS_ENABLED to false does not fix this issue since the error is thrown on the call to session_set_save_handler() on line 168, and that line is not guarded by sessions being enabled.

@danielbachhuber
Copy link
Contributor

So instead I went another direction, and this the fix to stop having it pollute the logs ends up being very simple; set session.use_cookies to '0' when WP CLI is running. Only the following change is required on line 144 of pantheon-sessions.php:

ini_set( 'session.use_cookies', defined( 'WP_CLI' ) && WP_CLI ? '0' : '1' );

If you agree I can either create a pull request, or given how simple it is maybe you can just apply it?

This seems like a reasonable approach to me. Landing it with #154

BTW, setting PANTHEON_SESSIONS_ENABLED to false does not fix this issue since the error is thrown on the call to session_set_save_handler() on line 168, and that line is not guarded by sessions being enabled.

Good to know!

@mikeschinkel
Copy link
Author

Thanks @danielbachhuber!

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