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

Alter site aliases for all command types #5273

Open
bomoko opened this issue Oct 13, 2022 · 15 comments
Open

Alter site aliases for all command types #5273

bomoko opened this issue Oct 13, 2022 · 15 comments

Comments

@bomoko
Copy link

bomoko commented Oct 13, 2022

Existing document

Please provide a link to the existing document that is unclear or incomplete.

What are you attempting to do

The example given in the documentation implementing a @hook pre-init works very well when running commands like drush @somealias cr. However, I'm not able to get it to work in the case of a command like sql-sync where the aliases are passed as arguments, it's not clear the right way to approach this.

We're in the position that different site aliases are dynamically created as environment come online, which means we can't have a fixed site alias file. The sites themselves also potentially have different ssh details, as they may be sitting on different clusters.

This may actually be a feature request if this isn't possible.

One potential route would be some mechanism to either wrap or alter the siteAliasManager itself. But any way of being able to manipulate the siteAliasManager before it's consumed by the command functions would be great.

In what way is the existing documentation unclear or incomplete

It's not clear how, with drush commands like drush sql-sync @sourcealias @targetalias we are able to manipulate the values given by the siteAliasManager

What should the documentation say instead?

As mentioned above, this may be a feature request rather than a documentation request - but ideally there would be some documentation demonstrating how to accomplish this.

Thank you so much for any information you might be able to provide.

@bomoko bomoko closed this as completed Oct 13, 2022
@bomoko
Copy link
Author

bomoko commented Oct 13, 2022

Apologies for closing, I thought I might have figured out a way to achieve it. If anyone could help, it would be really appreciated.

@bomoko bomoko reopened this Oct 13, 2022
@weitzman
Copy link
Member

In general this is not possible. Suggested approach is to have a command/script that rebuilds the list as needed (or even as a pre step before running a drush command). How would you dynamically determine the list of aliases and their details?

@weitzman
Copy link
Member

See #5225

@bomoko
Copy link
Author

bomoko commented Oct 13, 2022

@weitzman - thanks for the reply. I see we're looping around to an earlier Lagoon issue as well. We've changed the way that the ssh service in Lagoon works in order to support direct ssh'ing to a cluster, not proxied via a central service as it currently is.

To answer your question about how we would determine the list of aliases, I can show you the POC where I was testing out the pre-init hook here https://github.com/amazeeio/drupal-integrations/pull/9/files

You'll see that our environments are centrally managed and that we pull their ssh details via a graphql call.

We could certainly write an alias file as a pre-step before running a command. In the long run, being able to hook into the siteAliasManager would be very cool - but I'm happy to take what's possible.

@tobybellwood
Copy link

tobybellwood commented Oct 13, 2022

How can we dynamically tell drush to read from an alias file we create, if we can't control the drush command - assuming there is also a hook to read the alias file?

@weitzman
Copy link
Member

There are some default paths where drush looks for alias files, plus you can add more

  1. --alias-path cli option
  2. drush config. See https://www.drush.org/latest/using-drush-configuration/#specify-the-folders-to-search-for-drush-alias-files-siteyml.

@tobybellwood
Copy link

Hmm, will have to consider how much we can set any of these hardcoded files, switches or paths dynamically, hence the preference for using the drush internals. On a site-by-site basis it's easy, but doing it within our Drupal-integrations package is way preferable.

@weitzman
Copy link
Member

One more thing I'll mention in case its helpful is that Drush recognizes site specifications in addition to site alias names. See https://www.drush.org/latest/site-aliases/#site-specifications. So you could have a wrapper like foo drush core:status and then foo does the graphql and finally runs drush user@server/path/to/drupal#uri core:status. No site alias file ever gets written.

@bomoko
Copy link
Author

bomoko commented Oct 14, 2022

Thanks for that @weitzman - does makes sense, but before I run off and write a POC for this, does that address the case with sql-sync where we pass in aliases as arguments? That is, are we able to pass in multiple site specifications somehow, if not, I'm not entirely sure it's going to help.

Again, thank you for all of your help and insight on this. I'm thinking that we can likely close this particular issue.
The one thing I would say, though, is it would be really cool if it were possible to provide a custom implementation of the SiteAliasManager - I don't know how feasible that would be, but just having the opportunity to override and replace the class would likely sort out most all the problems we've seen in the various issues linked to around this.
I'm happy to try dig into this problem if you think it's worth a go.

@weitzman
Copy link
Member

weitzman commented Oct 14, 2022

Site specifications should work fine when passed as argument to SQL sync. The docs even say say.

I also think replacing the SAM would be handy here. Maybe @greg-1-anderson can say more about the feasibility of that. We havent had swappable services in Drush yet so it would be a pretty big change. The alias manager is already a service in the Drush container.

@greg-1-anderson
Copy link
Member

It's been a while since I've looked at this code. Isn't there an alias alter hook that you could attach to rather than using pre-init?

Regarding swapping out services, Drush isn't really designed for that. The DI container will provide references to a service to any object that requests one. By the time Drush is ready to run hooks, there are references to the site alias manager present in a number of files. The DI container doesn't keep a list (it just returns references; it never receives any info about who requested them, or which responses were cached). If you figured out all of the places where you had to fix up the site alias manager, that might work for the moment, but would be in danger of breaking at any time if another reference was added to the DI container.

We have talked about making some services indirect, e.g. a logger service that dispatches to the actual loggers, so that they could be dynamically altered by hooks. Do we want to do this for every service? Probably not.

Of all of the options, the one I would prefer would be adding hooks to the site alias manager, e.g. alias-not-found hook, populate additional alias fields hook, etc.; these would look like "register" / "addListener" methods on the site alias manager, with Drush code to find all of the site alias hooks and register them.

@weitzman
Copy link
Member

weitzman commented Oct 14, 2022

Alias alter example is at https://raw.githubusercontent.com/drush-ops/drush/11.x/examples/Commands/SiteAliasAlterCommands.php. I'm not sure if that would work for a remote dispatch (e.g. drush @foo status) or for a sql sync (e.g. drush sql:sync @foo @bar). Would need some investigation.

@greg-1-anderson
Copy link
Member

Hm, okay, so our alias alter example doesn't really alter the alias, it just examines the alias and sets global input options. I think that hook pre-init happens before redispatch, which I believe is just a hook init. I think it would be even better to have an actual alias alter hook. If no one does the work for that, then maybe the hook pre-init trick will work well enough.

@bomoko
Copy link
Author

bomoko commented Oct 16, 2022

@greg-1-anderson - I'm more than happy to take a swing at doing an alias alter hook.
Presumably this paragraph above gives the requisite design hints?
Of all of the options, the one I would prefer would be adding hooks to the site alias manager, e.g. alias-not-found hook, populate additional alias fields hook, etc.; these would look like "register" / "addListener" methods on the site alias manager, with Drush code to find all of the site alias hooks and register them.

@greg-1-anderson
Copy link
Member

@bomoko Yes; please let me know if you want any further clarification on any of my ideas.

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

No branches or pull requests

4 participants