MB-9804 update local setup and instructions #78

felipe-lee · 2021-10-22T01:21:05Z

Description

We want to make it easier for people to set up the project locally. To that end, I've implemented fresh-brew since we're testing it out in a few repos and it makes it much faster and easier to get going.

We also want to make this repo easier to maintain so we're dropping support for asdf and docker local setups in favor of pyenv and nix. This PR removes ASDF entirely because it was too intertwined with how pyenv worked. The docker instructions are still available, but are marked as unsupported.

Reviewer Notes

Should we just fully remove docker local setup? Like the instructions and Makefile commands?

Setup

Try out the setup steps outlined in the README and let me know if you have any issues.

Code Review Verification Steps

Request review from a member of a different team.
Have the Jira acceptance criteria been met for this change?

References

Jira story for this change.

…ASDF support Also clean up a few commands

…vated

Removed `ASDF` docs entirely because it relied on how the old `Markdown` file worked. Left `docker` setup, but marked as unsupported.

It's needed for the test utils from `locust`. Unfortunately `locust` is still using a deprecated way (`tests_require`) of defining their requirements for tests, which means we can't import them more easily (e.g. by listing it as `locust[test]` or something. Maybe in a future version we'll be able to do that rather than listing an individual requirement.

monfresh · 2021-10-22T14:06:01Z

This makes my heart sing! I'll give it a try in a bit.

monfresh · 2021-10-22T18:54:05Z

Install_tools worked fine, but at the end of make setup I got a warning about pip being out of date, and then an error related to nix:

WARNING: You are using pip version 21.2.4; however, version 21.3.1 is available.
You should consider upgrading via the '/Users/moncef/projects/transcom/milmove_load_testing/.direnv/python-3.9.7/bin/python3.9 -m pip install --upgrade pip' command.
make: *** No rule to make target `/nix/var/nix/profiles/milmove-load-testing/bin/pre-commit', needed by `.git/hooks/pre-commit'.  Stop.

monfresh · 2021-10-22T18:55:24Z

I think we need to say something about adding .nix-disable if you're going the pyenv route.

monfresh · 2021-10-22T18:59:18Z

Also, I use the fish shell and the command that was added to my config doesn't work in fish:

~/.config/fish/config.fish (line 32): $(...) is not supported. In fish, please use '(pyenv)'.
eval "$(pyenv virtualenv-init -)"
      ^
from sourcing file ~/.config/fish/config.fish
	called during startup
source: Error while reading file '/Users/moncef/.config/fish/config.fish'

Looking at the node setup, I think this is what is needed for fish:

status --is-interactive; and source (pyenv init - | psub)

monfresh · 2021-10-22T19:01:27Z

Okay, so after I modified the last 2 lines in my fish config to this:

status --is-interactive; and source (pyenv init - | psub)

status --is-interactive; and source (pyenv virtualenv-init - | psub)

and after opening a new terminal tab, I was finally able to run make setup without errors.

felipe-lee · 2021-10-26T00:31:30Z

Thanks for the review @monfresh! I've made those edits and will see about finding another fish user to test with.

ronaktruss

Both the nix and pyenv instructions LGTM!

ronaktruss · 2021-10-26T16:51:22Z

README.md

+    * [MilMove/Office domains](#milmoveoffice-domains)
+    * [Handling Rate Limiting](#handling-rate-limiting)
+    * [Metrics](#metrics)
+  * [References](#references)

 <!-- Added by: ronak, at: Wed Oct 20 10:24:39 EDT 2021 -->


I see that this line hasn't been modified. Is there a standardized way of updating the TOC? I have been running

./gh-md-toc --insert README.md

Maybe we need more documentation around this/add some more instructions in the comments?

Ah my bad, I missed that line. Yeah, I thought about adding that to the docs, or even adding a script or Makefile target that does it for us so we don't have to remember to do it the same way each time. i can add it since I'm already editing several things related to setup

ronaktruss · 2021-10-26T16:51:41Z

.gitignore

+Brewfile
+pour.sh
+Brewfile.local.lock.json
+Brewfile.lock.json


Nit: There are 5 files that end without new line characters

I can add a .editorconfig file to make sure we're more consistent

Oh wait, we already have one. Weird, idk why it didn't auto-fix these for me. Maybe I have it turned off in my editor for some reason.

Oh haha, just barely looked at it more closely. We actually haven't set it to insert final newlines. I'll update it.

ronaktruss

LGTM. Left a comment about spacing in the TOC if you want to look into it

ronaktruss · 2021-10-26T20:18:33Z

Makefile

@@ -131,6 +60,10 @@ pretty: ensure_venv  ## Prettify the code
 lint: ensure_venv  ## Run linting tests
 	flake8 .

+.PHONY: generate_readme_toc
+generate_readme_toc:  ## Re-generates and re-places the table of contents for the README.md
+	./gh-md-toc --insert README.md


Thanks for adding this and adding the package as part of the install script!

ronaktruss · 2021-10-26T20:23:49Z

README.md

+      * [Base Installation](#base-installation)
+         * [Setup: Pyenv](#setup-pyenv)
+         * [Setup: Nix](#setup-nix)
+      * [Installing Python Dependencies and Pre-commit Hooks](#installing-python-dependencies-and-pre-commit-hooks)
+      * [Unsupported Setup](#unsupported-setup)
+         * [Alternative Setup: Docker](#alternative-setup-docker)


I just realized that it is using 3 spaces for each indent level for some reason. I just did a cursory look and it seems like it should honor your editor settings (ref).

I tried fixing it, but it keeps generating with the 3 spaces. I thought it maybe was detecting a difference in spaces, but even after making the whole file follow 2 spaces (even tried with making the python do this, but didn't commit that in the end), and it still does 3 spaces. Very weird. I think we use a different tool on mymove, so maybe we can use that instead. Though that should be a future pr.

Trying to fix some bad spacing and test if gh-md-toc can actually honor our spacing or not.

Also add link to docs in case anyone else needs to edit it

felipe-lee added 5 commits October 21, 2021 18:06

MB-9804 Add files needed to use fresh-brew for easing local setup

0567c90

MB-9804 Update Makefile to use new installation scripts and remove …

39e3a51

…ASDF support Also clean up a few commands

MB-9804 Stop ignoring .python-version so that venv can be auto-acti…

4976f12

…vated

MB-9804 Update README.md with new setup instructions

bc0b80c

Removed `ASDF` docs entirely because it relied on how the old `Markdown` file worked. Left `docker` setup, but marked as unsupported.

felipe-lee added the A-Team Created by A-Team label Oct 22, 2021

felipe-lee requested review from eamahanna and a team October 22, 2021 01:21

felipe-lee added 3 commits October 25, 2021 16:51

MB-9804 Add a note about how to disable nix

4e3671a

MB-9804 Modify the commands for setting up pyenv in the fish shell

8cc1007

MB-9804 Add comment to setup target

09172d1

ronaktruss reviewed Oct 26, 2021

View reviewed changes

felipe-lee added 5 commits October 26, 2021 12:31

MB-9804 Update .editorconfig to add end newline & set default charset

1775df8

MB-9804 Re-generate README.md using --insert flag

81f092a

MB-9804 Add missing newlines to the ends of files

43ecbc5

MB-9804 Add .nix-disable and generated README.md.* files to .gitignore

97aa8e9

MB-9804 Add gh-md-toc to fresh-brew and add related make target

52f872d

ronaktruss self-requested a review October 26, 2021 20:13

ronaktruss approved these changes Oct 26, 2021

View reviewed changes

felipe-lee added 2 commits October 26, 2021 16:19

MB-9804 Reformat README.md (goland)

ba6e9e7

Trying to fix some bad spacing and test if gh-md-toc can actually honor our spacing or not.

MB-9804 Remove tab_width since it defaults to indent_size

d877e1e

Also add link to docs in case anyone else needs to edit it

felipe-lee merged commit c2ce7be into master Oct 26, 2021

felipe-lee deleted the MB-9804-update-local-setup-and-instructions branch October 26, 2021 23:37

felipe-lee mentioned this pull request Dec 2, 2021

MB-10464 Add reasons for why local docker setup isn't supported anymore #89

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MB-9804 update local setup and instructions #78

MB-9804 update local setup and instructions #78

felipe-lee commented Oct 22, 2021

monfresh commented Oct 22, 2021

monfresh commented Oct 22, 2021

monfresh commented Oct 22, 2021

monfresh commented Oct 22, 2021

monfresh commented Oct 22, 2021

felipe-lee commented Oct 26, 2021

ronaktruss left a comment

ronaktruss Oct 26, 2021

felipe-lee Oct 26, 2021

ronaktruss Oct 26, 2021

felipe-lee Oct 26, 2021 •

edited

Loading

felipe-lee Oct 26, 2021

felipe-lee Oct 26, 2021

ronaktruss left a comment

ronaktruss Oct 26, 2021

ronaktruss Oct 26, 2021

felipe-lee Oct 26, 2021

MB-9804 update local setup and instructions #78

MB-9804 update local setup and instructions #78

Conversation

felipe-lee commented Oct 22, 2021

Description

Reviewer Notes

Setup

Code Review Verification Steps

References

monfresh commented Oct 22, 2021

monfresh commented Oct 22, 2021

monfresh commented Oct 22, 2021

monfresh commented Oct 22, 2021

monfresh commented Oct 22, 2021

felipe-lee commented Oct 26, 2021

ronaktruss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felipe-lee Oct 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronaktruss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felipe-lee Oct 26, 2021 •

edited

Loading