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

sql-sync options isn't used from some.site.yml #3421

Open
hanoii opened this issue Feb 28, 2018 · 22 comments
Open

sql-sync options isn't used from some.site.yml #3421

hanoii opened this issue Feb 28, 2018 · 22 comments
Labels

Comments

@hanoii
Copy link
Contributor

hanoii commented Feb 28, 2018

This doesn't work

develop:
  command:
    sql:
      sync:
        options:
          source-dump: /tmp/dump

But this, does:

drush sql-sync @site.develop @self --source-dump=/tmp/dump

Drush version : 9.2.1

@greg-1-anderson
Copy link
Member

Did you try:

command:
  sql:
    sync:
      options:
        source-dump: /tmp/dump

@hanoii
Copy link
Contributor Author

hanoii commented Feb 28, 2018

@greg-1-anderson not sure I understand what you meant. I tried exactly that as I put in the first comment, or did you mean something else? you meant indenting it outside of the alias?

@greg-1-anderson
Copy link
Member

In your comment, develop: seems out of place. Try without that. I did not try to reproduce myself.

@greg-1-anderson
Copy link
Member

Oh, this is an alias. I was thinking this was a drush.yml file. Need to consider again.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Feb 28, 2018

So, drush @site.env command will inject the options from @site.env for command. I'll have to look and see what sql-sync does with alias arguments; it may be more limited.

@hanoii
Copy link
Contributor Author

hanoii commented Feb 28, 2018

sure.. yes it's an alias... it's odd though, at least based on what what example.site.yml says:

# - 'command': These options will only be set if the alias
#   is used with the specified command.  In the example below, the option
#   `--no-dump` will be selected whenever the @stage alias
#   is used in any of the following ways:
#     - `drush @stage sql-sync @self @live`
#     - `drush sql-sync @stage @live`
#     - `drush sql-sync @live @stage`
# stage:
#   uri: http://stage.example.com
#   root: /path/to/remote/drupal/root
#   host: mystagingserver.myisp.com
#   user: publisher
#   os: Linux
#   paths:
#    - files: sites/mydrupalsite.com/files
#    - custom: /my/custom/path
#   command:
#     sql:
#       sync:
#         options:
#           no-dump: true

it implies that what I am doing should work but it is not.

Awesome if you can look.

This actually became an issue working on platform.sh projects where most of the filesystem is readonly. This is something that wasn't an issue with previous version, i think it was because the dump was by default to /tmp (which is writable).

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Feb 28, 2018

Looking at the code, it appears that the documentation you quoted above was converted from Drush 8 (which does support this feature), but the code to handle this was never implemented in Drush 9.

I don't think it will be hard to fix.

@weitzman
Copy link
Member

weitzman commented Feb 28, 2018 via email

@hanoii
Copy link
Contributor Author

hanoii commented Feb 28, 2018

@weitzman I didn't copied it completely, it follows:

#   NOTE: Setting boolean options broke with Symfony 3. This will be fixed
#     in a future release. See: https://github.com/drush-ops/drush/issues/2956

:)

@jurgenhaas
Copy link
Contributor

Came across a similar issue, but with the drush.yml configuration file. The documentation states it should be coded like this:

command:
  sql:
    dump:
      options:
        sync:
          options:

which seems to be wrong. The correct version would be

command:
  sql:
    sync:
      options:

However, when I use the option structure-tables-key: 'common' this will be used correctly where the option skip-tables-key: 'common' is not being used at all. When I use --skip-tables-key=common as a CLI option, then it is working as expected.

Is this related here or do I have to open a new issue?

@greg-1-anderson
Copy link
Member

Hm, there isn't any reason I can think of why structure-tables-key should behave differently than skip-tables-key, but I haven't looked at the code to confirm. Could have something to do with how sql-sync calls sql-dump.

@greg-1-anderson
Copy link
Member

Or maybe sql-sync has not named all of the sql-dump options in its option list.

@jurgenhaas
Copy link
Contributor

If I remember correctly, this had already been a similar issue in Drush 8 and I just had been lazy to report it at the time and resolved it in my shell alias. But since those are no longer supported I'm now stuck with that issue and now wonder if it may be a left over issue from earlier versions?

@weitzman
Copy link
Member

weitzman commented Mar 2, 2018

There may be a bug here, but I can't think of a reason to store an option like structure-tables-key in an alias. Its not likely to vary by environment. Therefore, put it in a config file like /drush/drush.yml. At least thats what I do on my big Drush9 project.

@jurgenhaas
Copy link
Contributor

@weitzman yes, but: it doesnt work in the drush.yml file right now as I reported late in my comments. It's consistant and reproducable. Do you need more detail?

@weitzman
Copy link
Member

weitzman commented Mar 4, 2018

I did some testing, and am unable to reproduce this. Both --structure-tables-key and --skip-tables-key are being used by sql:dump. Note that these keys have to defined on sql:dump, not sql:sync. This is clarified in a new docs commit 7fbf122. Thanks for pointing out that this was malformed.

Let us know if the new docs help get this working.

@hanoii
Copy link
Contributor Author

hanoii commented Mar 4, 2018

I should point out that this issue somehow diverged in its topic. My initial issue is still there, which relates to being able to pass options to sql-sync through the alias file.

@jurgenhaas
Copy link
Contributor

@hanoii sorry for that, I wasn't meant to hijack your issue. It just felt as if it was related. But I guess that alias file support will no longer be a thing.

@weitzman I can confirm that it's now working for my case and the issue has been that I defined the options in drush.yml for the sync command and not for the dump command.

@weitzman
Copy link
Member

weitzman commented Mar 4, 2018

@hanoii - are you using 9.2.1 on both sides of the sql:sync? if so you shouldn't need to define a dump file. the name is randomly generated for you.

the issue here is that source-dump is looked up on the machine which is initiating the the sql:sync which is @self, not @develop. this may be different than prior versions of drush, which is what @greg-1-anderson is alluding to in #3421 (comment).

We need to fix docs or add back this feature. I'm ambivalent.

@weitzman weitzman changed the title sql-sync options doens't seem to be picked up from some.site.yml sql-sync options isn't used from some.site.yml Mar 4, 2018
@hanoii
Copy link
Contributor Author

hanoii commented Mar 4, 2018

@weitzman it's the exact same version on both sides. The problem is, as I mentioned on #3421 (comment) that the remote side in this case is read only, except for the usual drupal writable dirs and some others and it's now outputting in the home dir of the remote:

'result-file' => $options['source-dump'] ?: 'auto',

I think before it defaulted to /tmp.

--source-dump should be able to be provided to the source alias.

I guess ideally, you should be able to provide separate options on the alias for when you use it as source and dest

@greg-1-anderson
Copy link
Member

We need a site-alias API to adjust $options based on the provided alias parameters.

@weitzman
Copy link
Member

weitzman commented Mar 4, 2018

If HOME isn't writable on a host, you might as well point drush to a writable home. If possible, set HOME=/tmp. Thats an environment variable called HOME (see usage).

One technique for setting env variables is now documented in a PR at drupal-project. You can implement that PR on your own project, even before the PR is merged.

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

No branches or pull requests

4 participants