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

Hook system overhaul: Access any service, support other commands, user-defined hooks #1372

Closed
rfay opened this issue Jan 9, 2019 · 9 comments
Labels
Prioritized We expect to do this in an upcoming release

Comments

@rfay
Copy link
Member

rfay commented Jan 9, 2019

Is your feature request related to a problem? Please describe.

There are a number of oversights in our hook system and fixing them up would make a lot of people happy even if it disrupted their workflow a bit:

tomasnorre pushed a commit to tomasnorre/ddev that referenced this issue Jan 15, 2019
@wizonesolutions
Copy link
Contributor

I'm using post-start to kick off a Docker container on the host. It's a headless Chrome container; it's hard to get working within DDEV for now because it expects pages loaded through it to be requested by IP or localhost, and DDEV internal IPs are not stable, but the host.docker.internal IP is.

I run it -d --rm and would like to kill it "pre-rm" (which would also remove it), but there's no hook for that.

The hook would just be in config.yaml like now. My start one looks like:

hooks:
  post-start:
  - exec: touch ./-
  - exec-host: /bin/sh run-behat-requirements.sh

So I'd expect it to become:

hooks:
  post-start:
  - exec: touch ./-
  - exec-host: /bin/sh run-behat-requirements.sh
  pre-rm: # or maybe pre-stop if the other PR gets merged
  - exec-host: docker kill ddev-fps-chromeheadless

@rfay rfay added the Prioritized We expect to do this in an upcoming release label Apr 26, 2019
@rfay
Copy link
Member Author

rfay commented Apr 26, 2019

#1038 is more specific, about specifying target container for hook execution.

@rfay
Copy link
Member Author

rfay commented May 31, 2019

Overall there are so many things that can be done here:

  • exec task must be able to target container
  • User-defined commands that could be executed with ddev <command>, perhaps with or without arguments.
  • pre- and post- hooks for all commands, including user-defined commands
  • "Tasks" are currently "exec" and "exec-host". Could they be also any ddev command?

@cferthorney
Copy link

This has been discussed in Drupal Slack. I have summarised my thoughts below due to the 10K message limit on Slack.

I would like to be able to run custom scripts using (For example) ddev {script name} or ddev exec {script name} (at worst). This would mirror functionality already present in Docksal's custom commands which use bash. Docksal also has the concept of addons, which are centrally submitted scripts such as the phpunit addon by Oliver Davies. There is also Lando's custom tooling which puts commands in the yml. Specific examples of custom commands I have used/written for Docksal include:

  • dev-reset (A reset script I wrote to truncate the database, import configuration from the config/sync directory to completely reset the database for migration testing)
  • truncate-watchdog (Runs drush sql:query 'TRUNCATE watchdog;' to speed up finding specific errors)
  • core-test-setup (Runs the necessary scripting to execute the core PHPUnit test suite without needing to ddev ssh into the cli container).
  • compile-css (Lint and compile CSS ready for production use as opposed to development, or vice versa)

With regards to the possibility this type of functionality could use existing (or new) hooks; the above examples, I feel, do not fit well within (say) a post-startup hook as I may not always want to be able to run core tests, or may not always be migrating content. Some may also need running multiple times a coding session (Let alone a project sprint).

@rfay
Copy link
Member Author

rfay commented Jun 17, 2019

Thanks so much @cferthorney - that helps loads. I will try to take a look at docksal's approach and lando's custom tooling as well. But the specific examples really do help. Thanks!

@opdavies
Copy link

What @cferthorney said. 😆

@JustSteveKing
Copy link

I think what I would like to see here is a commands directory which is auto pulled into ddev? What this will allow us to do is configure and add commands much like we do with additional services in ddev, so we are sticking to an already defined pattern. Such as ddev phpunit would execute the ./.ddev/commands/command.phpunit.sh for example - an even better option would be to be able to dynamically check the operation script so that we could mix sh and php and even py files easily by following a convention .

@cferthorney
Copy link

I think what I would like to see here is a commands directory which is auto pulled into ddev? What this will allow us to do is configure and add commands much like we do with additional services in ddev, so we are sticking to an already defined pattern. Such as ddev phpunit would execute the ./.ddev/commands/command.phpunit.sh for example - an even better option would be to be able to dynamically check the operation script so that we could mix sh and php and even py files easily by following a convention .

This is effectively what Docksal do with the exception Docksal only uses Bash at present. I don't know if they have future plans to move to allowing other environments/languages. To my mind this would be perfect for Ddev, but I also realise I am slightly biased having used the docksal version extensively over the last 18 months to 2 years.

Also @rfay any script in the commands folder would have access to all the environment variables Ddev provides to/with Docker too. This way I could do something like drush uli --url=${VIRTUALHOST} --no-browser --uid=1 or something similar as a ddev command etc. I think any "command" option that doesn't provide access to ddev's own env. variables is going to be very limited.

rfay added a commit that referenced this issue Jun 24, 2019
…1038 (#1634)

* Switch to yaml v3
* Add composer task
* Add multiple pre- and post- hooks
* Allow exec hook to specify container to act on
@rfay rfay added this to the v1.10 milestone Jun 28, 2019
@rfay
Copy link
Member Author

rfay commented Jun 28, 2019

The remaining work here is custom commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prioritized We expect to do this in an upcoming release
Projects
None yet
Development

No branches or pull requests

5 participants