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

Site install interactive #2485

Closed

Conversation

serundeputy
Copy link
Contributor

@greg-1-anderson
Copy link
Member

Yeah, this looks good in concept -- but why isn't the results from Travis showing up here? I looked briefly and did not see the tests. If you can demonstrate that the tests are passing, then I'd be okay with merging this, porting to 8.x, and then doing the annotated conversion as a follow-on step.

If this were to be done to the annotated version of this command, @hook interact would be the thing to use to get the database credentials.

@greg-1-anderson
Copy link
Member

Tests all green: +1

@weitzman
Copy link
Member

I just merged #2466, so this needs a re-roll.

  1. I'm not sure we can use hook @interact after all. If user is re-installing, he/she doesn't need to provide a db-url, and we shouldn't ask for one. We bootstrap to SITE level during 'pre' phase so I we know yet whether a db-url is needed during the interact phase. So, I think the code in pre phase like the PR is now.
  2. We need to replace call to drush_set_error() with an exception and use $options instead of drush_get_option()

@weitzman
Copy link
Member

weitzman commented Feb 8, 2017

ping @serundeputy

@serundeputy
Copy link
Contributor Author

@weitzman thanks for the ping.

I've refactored this agains the new master and I've got the prompts working but I'm confused about what $sql = SqlBase::create($commandData->input()->getOptions()); is expecting.

I've set the options via prompts but it is bailing out with unrecoverable errors.

Can you provide any insight?

thanks,
~Geoff

@weitzman
Copy link
Member

The only setOption() that you should need is on db-url. You can use regular local variables to briefly store stuff like password, host, etc.

When you pass $commandData->input()->getOptions() to SqlBase::create(), its important that 'db-url' is set in the returned array.

What errors are you seeing?

Issue drush-ops#2484: Give site-install and interactive mode.

Issue drush-ops#2484: Give site-install and interactive mode.
@serundeputy
Copy link
Contributor Author

Thanks @weitzman
refactored the PR tests are passing :)

Manually tested on current D8 8.2.6 works great.

Manually tested on current D7 7.54 but si is not working on master for D7 at all:

[warning] require(/Users/geoff/sites/flat/drupal-7.54/autoload.php): failed to open stream: No such file or directory drupal.inc:17

I think the D7 is a separate issue though.

~Geoff

@weitzman
Copy link
Member

I moved the code around and then added this in 820fb5d. Thanks @serundeputy. Great to have some more interactivity in Drush.

@weitzman weitzman closed this Feb 14, 2017
@greg-1-anderson
Copy link
Member

This could also have been done with @hook interact

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.

None yet

3 participants