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

vvv-hosts for utilities #1657

Open
wants to merge 1 commit into
base: develop
from

Conversation

Projects
None yet
2 participants
@Mte90
Contributor

Mte90 commented Oct 24, 2018

Summary:

Add the support of scanning inside utilities to check for domains

Checks

  • I've tested this PR with Vagrant v2.2 and VirtualBox vXX on Linux
  • This PR is for the develop branch not the master branch
  • I've updated the changelog
  • This PR is complete and ready for review

@Mte90 Mte90 requested review from tomjn and LoreleiAurora Oct 24, 2018

@tomjn

This comment has been minimized.

Member

tomjn commented Oct 24, 2018

I don't think a scan is necessary, especially since this will add hosts for utilities that aren't being used.

Since we already know which utilities are active, we should just loop through them and assume the location of vvv_hosts which will be faster, the same way that we load the provision.sh

@tomjn tomjn added the enhancement label Oct 24, 2018

@Mte90 Mte90 referenced this pull request Oct 24, 2018

Open

Tideways with Xhgui #18

@Mte90

This comment has been minimized.

Contributor

Mte90 commented Oct 24, 2018

Well if we are talking of performance this doesn't change a lot, check only if the tool inside that folder have the files but this will apply also to the tools that are not enabled and this is not good indeed :-/

The problem of using the utilities is that they can have a different folder name compared to the utilities name. Right now the folder for the tideways is xhgui but the tool is another one because xhgui is the UI.
With this implementation we ignore this problem.

@tomjn

This comment has been minimized.

Member

tomjn commented Oct 24, 2018

The problem of using the utilities is that they can have a different folder name compared to the utilities name. Right now the folder for the tideways is xhgui but the tool is another one because xhgui is the UI.
With this implementation we ignore this problem.

I don't follow? If we load php70 in the core utility then that means core/php70/provision.sh gets executed, so why not skip the scan and then load core/php70/vvv_hosts if it exists?

A scan would load all utilities hosts files, which could be problematic, especially if some utilities are alternatives to each other, and implement the same thing with shared hosts. That and I can see people misusing it by nesting folders.

I'm also keen for things to be as fast as they can given that we have to consider time to provision, I'd like to be able to parallelise things in the future, and I'm also keen to deprecate vvv_hosts scanning in favour of the config file and having the file in a specific place, ideally provision/vvv_hosts and load it the same way vvv-nginx.conf and vvv-init.sh are loaded

@Mte90

This comment has been minimized.

Contributor

Mte90 commented Oct 24, 2018

I misunderstood because I have a file provision.sh in the folder so I added the file on the utilities folder not on the global one, so right now the implementation check inside /var/www/default/[tool] for the file.

@tomjn

This comment has been minimized.

Member

tomjn commented Oct 24, 2018

I'm keen for everything to have a proper place, and since this is something we're adding I'd prefer VVV to check in that proper place rather than search for the file.

So for a utility, a folder should contain everything, so for tideways I would expect a tideways/provision.sh and a tideways/vvv-hosts. One could even adjust the VVV utility provisioner so that vvv-nginx.conf is loaded into the appropriate place automatically.

The important part being that there is only 1 correct place and answer, and only what's enabled gets loaded. No scans, no multiple correct places, strict simple and clear

@tomjn

This comment has been minimized.

Member

tomjn commented Oct 24, 2018

Speaking of which, it would be good to adjust things so that vvv-init.sh can be used instead of provision.sh so that the mental model of site templates can be reused and things make more sense

@Mte90

This comment has been minimized.

Contributor

Mte90 commented Oct 24, 2018

So you want that VVV itself to some standard stuff like move this files of every utilities:

  • vvv-hosts to /VVV/provision (with merge of an existing one)
  • vvv-nginx.conf to /etc/nginx/custom-utilities (inside virtual machine)

Also:

  • execute vvv-init.sh instead provision.sh for utilities (inside the virtual machine)
  • remove the code of scanning and use the path of the global vvv-hosts

Just to understand what I have to do :-)

@tomjn

This comment has been minimized.

Member

tomjn commented Oct 24, 2018

@tomjn

This comment has been minimized.

Member

tomjn commented Oct 24, 2018

@tomjn tomjn referenced this pull request Oct 24, 2018

Open

Utility hosts #1659

1 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment