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

[dev/core#750] Don't check server variables if we're running in CLI #17636

Merged
merged 1 commit into from
Jul 9, 2020
Merged

[dev/core#750] Don't check server variables if we're running in CLI #17636

merged 1 commit into from
Jul 9, 2020

Conversation

homotechsual
Copy link
Contributor

Overview

This PR introduces skipping the server variable checks if we're running in a CLI environment, removing an error when running Drush commands against Drupal 8 and Drupal 9 based sites.

Before

What is the old user-interface or technical-contract (as appropriate)?
Drush commands error due to missing server variables, however we shouldn't actually check these variables when running from CLI environments as it's not reasonable to expect them to be set.

After

What changed? What is new old user-interface or technical-contract?
The server variables check is now only run for non-cli environments. The error no longer appears!

@civibot
Copy link

civibot bot commented Jun 16, 2020

(Standard links)

@civibot civibot bot added the master label Jun 16, 2020
This PR introduces skipping the server variable checks if we're running in a CLI environment, removing an error when running Drush commands against Drupal 8 and Drupal 9 based sites.
Fix whitespace errors.
Remove variable comment.
Fix more whitespace!
@seamuslee001
Copy link
Contributor

@mikeyMJCO I tested this and running a straight drush updb still left me with

 [warning] CiviCRM: System: You have -1 allocated (minimum 32Mb, recommended 64Mb)
 [error]  CiviCRM: System: Please specify a realistic site URL (Ex: drush -l http://example.com:456 ...).

which seems to come from https://github.com/civicrm/civicrm-core/blob/master/setup/plugins/checkRequirements/CheckDrushBaseUrl.civi-setup.php#L23 but otherwise this works

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 'otherwise this works' -= "better in than out?"

@seamuslee001
Copy link
Contributor

yes merging

@seamuslee001 seamuslee001 merged commit f7d3b28 into civicrm:master Jul 9, 2020
@Rar9
Copy link

Rar9 commented Aug 13, 2020

I'm on civicrm 5.30alpa and i still see this error on drupal 9

@fsnet
Copy link

fsnet commented Jan 19, 2021

Same here: CiviCRM 5.33.2 / Drupal 9.1.2

@homotechsual
Copy link
Contributor Author

This is a merged/closed PR - please open an issue on https://lab.civicrm.org if you're still seeing this issue which relates ONLY to warnings shown in CLI tools (e.g Drush).

For any other issues you can open an issue on GitLab but it's not likely to be related to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants