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

Load env changes with single make command #247

Merged
merged 5 commits into from
May 4, 2022

Conversation

DonRichards
Copy link
Member

Keeping with the idea of helping the user from needing to know/use docker/docker-compose commands and simplifying the process.

This adds make env to fetch the changes made to the .env file post-build.

To test

Run make clean to start with a clean environment then run make env

It should fail with a message "No .env file found." and exit with doing nothing to the system.

  • Now run make local to build the system. This will automatically create a .env file.
  • Navigate to http://islandora.traefik.me:8983/solr/#/ and this should give you a 404. Watch out! Your browser might try to redirect you to the HTTPS equivalent. This will not work. Switch it back to HTTP manually.
  • Modify the .env file to expose solr
  • Run make help and the output should include " env _ Pull in changes to the .env file.`
  • Run make env and navigate to http://islandora.traefik.me:8983/solr/#/

Also added some text formatting to the help definitions.

@rosiel rosiel requested a review from ysuarez April 13, 2022 17:34
@ysuarez
Copy link
Contributor

ysuarez commented Apr 26, 2022

(Here are my initial tests results. I will do more testing later and add another comment.)

A) I tested this PR and did not get make env to show the "No .env file found" message because lines 3-7 of the Makefile always create an empty .env file if one is missing every time the Makefile runs...

isle-dc/Makefile

Lines 3 to 7 in 189b0be

ENV_FILE=$(shell \
if [ ! -f .env ]; then \
cp sample.env .env; \
fi; \
echo .env)

If I comment out lines 3-7 then I do get the expected error message.

B) the text formatting to the help definitions looks great!

@ysuarez
Copy link
Contributor

ysuarez commented Apr 26, 2022

Here is the second part of my testing, and the PR worked as expected.

  • I ran make local to build the system, and got the expected Solr page 404.
  • Modified the .env file to expose solr
  • Ran make env and navigated to http://islandora.traefik.me:8983/solr/#/ and saw the Solr admin page. (Though there were no cores.)

image

There is a small potential bug when running make clean then make make env, but besides that this PR works fine.

@DonRichards
Copy link
Member Author

@ysuarez It should never create an empty .env file. It's supposed to copy the sample.env file. And yes, that no cores thing is a make local issue/feature.

Copy link
Contributor

@ysuarez ysuarez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR works as expected. I think it should be committed.

BTW Don, I wrote something wrong in an earlier comment. I didn't mean to say we would get an empty .env file, I meant that with one of the suggested testing instructions of running make clean then make env, I would not end up with a missing .env to trigger the warning .env error message in the PR that the file was missing. This is because currently whenever any make command is run as you mentioned, lines 3-7 of the Makefile will automatically create an .env that is a copy of the sample.env. I think it is a good defensive practice by this PR to be thorough in first checking if the .env exists, before doing its work.

@seth-shaw-unlv seth-shaw-unlv merged commit be7e0db into development May 4, 2022
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