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

Adding environment variable files in lando. #42

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mohit-rocks
Copy link
Contributor

Currently DRUSH_OPTIONS_URI is set in drush.yml file.
This is causing issues with hosts like platform.sh where they are reading this file as well.
For now we can use environment variables in .lando directory so it remains active for local only.
Another use case is when we configure blackfire etc. we need to add environment variables for those as well.
Having seperate environment variable file specific to local lando can be useful for those cases.

Currently DRUSH_OPTIONS_URI  is set in drush.yml file.
This is causing issues with hosts like platform.sh where they are reading this file as well.
For now we can use environment variables in .lando directory so it remains active for local only.
Another use case is when we configure blackfire etc. we need to add environment variables for those as well.
Having seperate environment variable file specific to local lando can be useful for those cases.
@hussainweb
Copy link
Member

@mohit-rocks, thanks for the PR.

I am not clear on what you're fixing here. You said that DRUSH_OPTIONS_URI is set in drush.yml, but it is commented out. How is that commented out line causing a problem with hosts?

I also don't understand the changes where you are creating a .lando/defaults.env. What's the problem with the root-level .env file? That file shouldn't be a part of the git repository and hence it won't be deployed to any platform. Is this causing a conflict in some other way?

@hussainweb
Copy link
Member

hussainweb commented Mar 18, 2021

I think I see the issue now (or at least a part of it). The problem is that the hosts use the .drush.yml file to set the URI and because it is Lando specific, it fails. Is that correct?

If so, I don't think the current fix is long-term. Loading the URI using an environment variable is fine but it will only apply to Lando context (i.e. when running lando drush). In that case, we don't strictly need to set the URI. I was hoping we would extend this to a case where we could run drush from outside Lando but there are other challenges to that (I was exploring this in consolidation/site-process#52). Let's take some time to think of a cleaner solution here.

As for the environment variables, I agree it is valuable but that works even right now with a root-level .env file. It does so at the Drupal level by using the vlucas/phpdotenv package which takes effect even on deployment targets, not just Lando. This is useful if you want to host your site other than the common PaaS platforms. And .env is a common convention across languages and frameworks and there is value in sticking to that here. If we want to move the DRUSH_OPTIONS_URI to an environment variable, maybe move it to .env.example?

@mohit-rocks
Copy link
Contributor Author

I was hoping we would extend this to a case where we could run drush from outside Lando but there are other challenges to that (I was exploring this in consolidation/site-process#52). Let's take some time to think of a cleaner solution here.

+1 to this. Let's review and think about more viable approach.

Apart from running outside lando, my main intention was to provide an option for commands like lando drush uli
This command always returns http://default as URI when proper options are not set.

As per the documentation of drush, it is suggested to use environment variable files.

@tormi
Copy link

tormi commented Apr 3, 2021

Apart from running outside lando, my main intention was to provide an option for commands like lando drush uli
This command always returns http://default as URI when proper options are not set.

As per the documentation of drush, it is suggested to use environment variable files.

Why not using Drush aliases like lando drush @local uli?

@hussainweb
Copy link
Member

@tormi, thanks. Do you find that command works as expected (i.e., does it give the proper URL instead of http://default?)

@tormi
Copy link

tormi commented Apr 5, 2021

@mohit-rocks
Copy link
Contributor Author

That's interesting @tormi @hussainweb , let me try it out and refactor this PR then.

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

Successfully merging this pull request may close these issues.

3 participants