Clarify Local Hosting Documentation by wtokumaru · Pull Request #44 · chronoscio/backend

wtokumaru · 2018-10-07T03:17:00Z

Took the liberty to modify the documentation to be more clear (to me at least). I think that we want to err on the side of excessive hand-holding and completeness to minimize inadvertent configuration hazing.

Also added several small comments and linting changes in other files which I can remove if they are too intrusive. I intend to remove and isolate to separate PRs anything particularly controversial.

wtokumaru · 2018-10-07T03:17:52Z

Does this repo not have an option to request reviews and assign relevant developers? It also would not let me select any labels before submitting. I suspect it is a permissions thing since I still can not add any after submitting.

quorth0n · 2018-10-07T03:42:21Z

.autopep8.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python3
+# TODO: Explain usage here?  Why do we need both pylint and pep8?


PEP8 is a Python formatting guideline. Autopep8 is a tool to automatically format python code according to the PEP8 standards. Pylint is a tool to lint python code.

Okay, pep8 for format, separately from linting.

wtokumaru

Some explanation.

wtokumaru · 2018-10-07T03:40:45Z

.autopep8.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python3
+# TODO: Explain usage here?  Why do we need both pylint and pep8?


I assume that this all happens through CI but is it also possible to run these directly?

Yes, they can be executed normally. VSCode is supposed to call them on save as well but it's not entirely working, see #32

.autopep8.py

wtokumaru · 2018-10-07T03:41:58Z

django.env.sample

-AUTH0_CLIENT_SECRET={SECRET}
-API_IDENTIFIER={IDENTIFIER}
+AUTH0_DOMAIN=chronoscio.auth0.com
+API_IDENTIFIER=https://chronoscio.org/api/


Included defaults here as well because we can.

wtokumaru · 2018-10-07T03:43:04Z

project/interactivemap/settings.py

    'interactivemap-frontend-*.now.sh',
 )

-AUTH0_DOMAIN = os.environ.get('AUTH0_DOMAIN', 'chronoscio.auth0.com')


Removed this default because it does not seem to work when combined with empty strings.

I don't know what you mean

Traceback (most recent call last): File "project/manage.py", line 29, in <module> execute_from_command_line(sys.argv) File "/home/tokumaru/.local/lib/python3.5/site-packages/django/core/management/__init__.py", line 371, in execute_from_command_line utility.execute() File "/home/tokumaru/.local/lib/python3.5/site-packages/django/core/management/__init__.py", line 317, in execute settings.INSTALLED_APPS File "/home/tokumaru/.local/lib/python3.5/site-packages/django/conf/__init__.py", line 56, in __getattr__ self._setup(name) File "/home/tokumaru/.local/lib/python3.5/site-packages/django/conf/__init__.py", line 43, in _setup self._wrapped = Settings(settings_module) File "/home/tokumaru/.local/lib/python3.5/site-packages/django/conf/__init__.py", line 106, in __init__ mod = importlib.import_module(self.SETTINGS_MODULE) File "/usr/lib/python3.5/importlib/__init__.py", line 126, in import_module return _bootstrap._gcd_import(name[level:], package, level) File "<frozen importlib._bootstrap>", line 986, in _gcd_import File "<frozen importlib._bootstrap>", line 969, in _find_and_load File "<frozen importlib._bootstrap>", line 958, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 673, in _load_unlocked File "<frozen importlib._bootstrap_external>", line 665, in exec_module File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed File "/home/tokumaru/chronoscio/backend/project/interactivemap/settings.py", line 174, in <module> jwks = json.loads(jsonurl.read()) File "/usr/lib/python3.5/json/__init__.py", line 312, in loads s.__class__.__name__)) TypeError: the JSON object must be str, not 'bytes'

I traced it back to precisely this one-line change and checked several times why flipping back and forth between line options. It happens if AUTH0_DOMAIN is 'chronoscio.auth0.com' while the other two are empty strings or None. However, if all 3 are None, it works fine. Might only be a problem when hosting locally.

What are the exact values of your environment variables?

The edge case I am describing is when you have none of the django.env sourced. When that happens it can still work with the defaults specified in settings.py except for this. When all the env vars are set, there is no need for defaults. The problem I observed is that the url is not compatible with empty strings or Nones, but if all three are None then they are compatible.

wtokumaru · 2018-10-07T03:43:07Z

project/interactivemap/settings.py

-AUTH0_CLIENT_ID = os.environ.get('AUTH0_CLIENT_ID', '')
-AUTH0_CLIENT_SECRET = os.environ.get('AUTH0_CLIENT_SECRET', '')
+AUTH0_CLIENT_ID = os.environ.get('AUTH0_CLIENT_ID')
+AUTH0_CLIENT_SECRET = os.environ.get('AUTH0_CLIENT_SECRET')


All three being None though seems to work.

quorth0n · 2018-10-07T03:43:15Z

.pylint.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python3
+# TODO: Explain usage here?  Why do we need both pylint and pep8?


README.md

wtokumaru · 2018-10-07T04:06:21Z

The Travis build failed in the same way that this fails when I try to use Docker, suggesting the problem is in the code rather than with my computer. I assume that master usually passes, so which change here makes a difference?

quorth0n

CONTRIBUTING.md must be placed in root, it's a GitHub specific thing. Maybe we could make a symlink or something.

quorth0n · 2018-10-07T03:49:18Z

README.md

+AUTH0_CLIENT_ID={Client ID}
+AUTH0_CLIENT_SECRET={Client Secret}
+```
+4) Install and run [Postgresql](https://www.postgresql.org/docs/9.3/static/tutorial-install.html) (AKA postgres) locally. You should not need to worry about creating any accounts or doing any setup but you may need to debug OS-specific problems that appear. For example, you may need to install postgis as well. (TODO: Test this on wiped setups to find out specific steps to do this properly.)


I think this is a little more verbose then necessary. A link should probably be provided to the geodjango setup information. Account handling is up to the user and OS-specific problems are a given so I don't think this sentence is necessary.

What is the geodjango setup needed for?What link in particular? I do not remember doing it but may have forgotten.

quorth0n · 2018-10-07T03:50:07Z

README.md

+AUTH0_CLIENT_SECRET={Client Secret}
+```
+4) Install and run [Postgresql](https://www.postgresql.org/docs/9.3/static/tutorial-install.html) (AKA postgres) locally. You should not need to worry about creating any accounts or doing any setup but you may need to debug OS-specific problems that appear. For example, you may need to install postgis as well. (TODO: Test this on wiped setups to find out specific steps to do this properly.)
+5) Change the HOST variable in the [settings.py](https://github.com/chronoscio/backend/blob/master/project/interactivemap/settings.py) file to be `localhost` instead of `db`. You may also need to change the PORT variable to match whatever you have set for postgis (TODO: investigate this).


s/postgis/postgres

That would make sense but the weird thing here is that the final line I had to run to get everything working for me was actually postgis. O_o
See: https://stackoverflow.com/questions/41666196/how-to-change-install-postgis-location-postgres
The answer here sudo apt-get install postgresql-9.6-postgis-scripts did not work for me but the comment on the answer that did more brute force sudo apt install postgis did work. However, you can see from the next comment that this is not the best way to solve the problem and is kind of overkill so I did not want to include it in the docs.

For geodjango you shouldn't need to install anything past postgresql-x.x, postgresql-x.x-postgis, postgresql-server-dev-x.x, python-psycopg2 according to the documentation.

quorth0n · 2018-10-07T03:52:30Z

README.md

+```
+7) Migrate and run the server with django.
+```bash
+# Migrate the project.


Run SQL migrations*

quorth0n · 2018-10-07T03:53:45Z

README.md

+
+project/api: REST API and a test file (how/why do we run the test)?  Defines the Nation, Territory, and DiplomaticRelation classes, which are our primary database items.
+
+project/interactivemap: Django settings and website directory layout?


I don't think documentation here is necessary past the project/ folder as it is standard django architecture.

What exactly is standard vs. custom? Which files exactly? If we have a standard directory with a custom subdirectory in it I would like to make it clear what is going on.

quorth0n · 2018-10-07T03:54:17Z

README.md

+
+project/interactivemap: Django settings and website directory layout?
+
+project: Django execution wrapper.


Django application directory*

quorth0n · 2018-10-07T04:06:18Z

docs/DEVELOPER.md

-2. Create Application > All Scopes > Authorize
-3. See the `client_id`, `client_secret`, and `audience` variables in the example cURL
+1. Log into https://manage.auth0.com/#/applications. It does not matter what names you select.
+2. Create Application > All Scopes > Authorize. Or just create a SPA (Single Page Application).


I'm going to have to look into this for the proper procedure. See #39.

Good to know. I had not gotten around to reading that issue yet.

quorth0n · 2018-10-07T04:06:41Z

project/interactivemap/settings.py

    'interactivemap-frontend-*.now.sh',
 )

-AUTH0_DOMAIN = os.environ.get('AUTH0_DOMAIN', 'chronoscio.auth0.com')


I don't know what you mean

quorth0n · 2018-10-07T04:06:54Z

project/interactivemap/settings.py

-AUTH0_CLIENT_ID = os.environ.get('AUTH0_CLIENT_ID', '')
-AUTH0_CLIENT_SECRET = os.environ.get('AUTH0_CLIENT_SECRET', '')
+AUTH0_CLIENT_ID = os.environ.get('AUTH0_CLIENT_ID')
+AUTH0_CLIENT_SECRET = os.environ.get('AUTH0_CLIENT_SECRET')


quorth0n · 2018-10-07T04:08:16Z

Does this repo not have an option to request reviews and assign relevant developers? It also would not let me select any labels before submitting. I suspect it is a permissions thing since I still can not add any after submitting.

It's a permission thing.

wtokumaru · 2018-10-07T04:15:58Z

CONTRIBUTING.md must be placed in root, it's a GitHub specific thing. Maybe we could make a symlink or something.

Weird, I never saw this as a problem in other projects. How do you know this? Is it just to make a pop-up when submitting a PR?

quorth0n · 2018-10-07T04:51:45Z

Never mind actually, I just checked GitHub's docs, they say it can be placed in the docs/ directory as well.

quorth0n · 2018-10-07T15:23:50Z

README.md

+AUTH0_CLIENT_ID={Client ID}
+AUTH0_CLIENT_SECRET={Client Secret}
+```
+4) Install and run [Postgresql](https://www.postgresql.org/docs/9.3/static/tutorial-install.html) (AKA postgres) locally. You should not need to worry about creating any accounts. You will also need to make sure you have set up [geodjango](https://docs.djangoproject.com/en/2.1/ref/contrib/gis/install/).  (TODO: Test this on wiped setups to find out specific steps to do this properly.)


This documentation link should refer to .../en/stable/contrib/... so that it's not version specific.

quorth0n · 2018-10-07T15:28:49Z

django.env.sample

-API_IDENTIFIER={IDENTIFIER}
+AUTH0_DOMAIN={PROJECT.auth0.com}
+AUTH0_CLIENT_ID={Client ID}
+AUTH0_CLIENT_SECRET={Client Secret}


Why was API identifier removed? Also, the sed regex in .travis.yml looks for specific values for these fields which it replaces with our secrets so that will have to be updated as well.

I just separated it elsewhere in the file to look nicer. Will look into updating .travis.yml.

Going to swap it back to {IDENTIFIER} I suppose for Travis to be the same, though it should not be necessary to include there? Why are some of these set with defaults in the sample env but others set with defaults in settings.py and/or .travis/yml? It seems kind of arbitrary. It feels more natural to be to just have one set of defaults and maaaaybe a second set of test-specific defaults but not 3 separate ones.

Oh sorry, didn't catch that. It's an Auth0-specific thing so I think it'd be better placed in the Auth0 group. I agree about the defaults actually, let's just keep them all here for simplicity's sake. The only test specific defaults are so that all third party libs can use our access tokens and such which is why they aren't publicly displayed.

amaury1093 · 2018-10-08T12:16:32Z

@wtokumaru About the permissions thing, I added you to the repo some time ago, have a look at https://github.com/chronoscio to accept it

wtokumaru · 2018-10-08T20:14:21Z

Should I be able to assign, request reviews, and add tags now? I still do not appear to be able to do nay despite the member identification.

wtokumaru · 2018-10-08T20:22:04Z

The Travis build failed in the same way that this fails when I try to use Docker,

This was probably due to me not updating the /travis.yml so I am looking into that.
Edit: Updated by following commits.

wtokumaru · 2018-10-08T22:34:48Z

Weird, the CI just went from working to not, without adding any commits, anyone have an idea why? Does it just have that much lag?

quorth0n · 2018-10-08T22:47:53Z

My guess would be that it went from "in progress" to failing. Regardless the reason is because of the changes to this line:

backend/django.env.sample

Line 6 in 1544696

API_IDENTIFIER=https://chronoscio.org/api/

.travis.yml needs to be updated for the sed replacement to work:

backend/.travis.yml

Line 15 in 1544696

- sed -i 's#{IDENTIFIER}#https://chronoscio.org/api/#g' ./django.env

I think that as we established it has to be the frontend application's client ID (see my recent posts in #39) we should keep it as {IDENTIFIER}.

dwaxe · 2018-10-09T03:41:10Z

README.md

 # ChronoScio Backend [![Build Status](https://travis-ci.org/chronoscio/backend.svg?branch=master)](https://travis-ci.org/interactivemap/backend) [![Codacy Badge](https://api.codacy.com/project/badge/Grade/0074e97bc13b476ea3eec279483d3cab)](https://www.codacy.com/app/whirish/backend?utm_source=github.com&amp;utm_medium=referral&amp;utm_content=interactivemap/backend&amp;utm_campaign=Badge_Grade)

 ## Getting Started
+We normally use [Docker](https://en.wikipedia.org/wiki/Docker_(software)) to simplify installation and configuration. Docker makes virtual "containers" as images which allow us to isolate code we run and work around computer-specific weirdnesses. Docker should work on [Linux](https://docs.docker.com/install/linux/docker-ce/ubuntu/) or [macOS](https://docs.docker.com/docker-for-mac/install/) but requires Hyper-V, which is not available on Windows 10 Home edition. If you have trouble with setting up Docker, see the **Local Development** instructions below instead. Otherwise, it should be simple to get started:


Mention that it does works on Windows 10 Pro 64 bit.

dwaxe · 2018-10-09T03:41:53Z

README.md

 # Navigate to http://localhost/, if you get a 502 error postgres likely has not been initialized yet,
 #   try again in a few seconds
 ```
+Use `docker-compose down` to stop hosting when you are done.


or just make stop or make clean (rebase this pr off latest master)

README.md

…essarily an sh file.

quorth0n · 2018-10-23T03:58:37Z

Could you try adding a cat django.env line to your travis.yml so we can ensure its values are correct?

amaury1093 · 2018-10-23T08:28:28Z

@whirish Are you sure about that? Our travis builds are public, anyone can then see our Auth0 credentials

quorth0n · 2018-10-23T19:22:15Z

@amaurymartiny Doesn't Travis filter out secrets when they're printed? I was thinking worst case we could just delete the build log

quorth0n reviewed Oct 7, 2018

View reviewed changes

wtokumaru commented Oct 7, 2018

View reviewed changes

quorth0n reviewed Oct 7, 2018

View reviewed changes

README.md Outdated Show resolved Hide resolved

quorth0n reviewed Oct 7, 2018

View reviewed changes

quorth0n added status/await kind/documentation labels Oct 7, 2018

quorth0n assigned wtokumaru Oct 7, 2018

quorth0n mentioned this pull request Oct 8, 2018

Make an authenticated request to the API #39

Closed

dwaxe reviewed Oct 9, 2018

View reviewed changes

README.md Show resolved Hide resolved

wtokumaru added 6 commits October 13, 2018 14:44

Move CONTRIBUTING.md to docs/.

69c623e

Minor context comments on hidden files.

f04a1a2

Fix links for contributing at bottom of README.

0a95b22

Add brief summary of codebase layout to README.

a3c77b2

Move Local Development instructions to README and embellish them.

59afcbb

Add more detail about Local Development.

70650eb

wtokumaru added 9 commits October 13, 2018 14:54

Clarify Auth0 usage when hosting locally.

3e0d9a0

Revise lint/format meta comments.

be78185

Request user to set auth0 domain too.

86985af

Modify clarifications to be a bit less wordy.

6894f5f

Tentatively put bash for quality standards though the file is not nec…

b45f548

…essarily an sh file.

Update url, .travis.yml, and other tweaks reqested in review.

4068a05

Undo changes to settings.py for now for CI.

3a3ee8d

Update README for undoing changes to .travis.yml and settings.py.

a76de9b

Resolve style file merge conflicts.

830f49a

		@@ -1,4 +1,5 @@
		#!/usr/bin/env python3
		# TODO: Explain usage here? Why do we need both pylint and pep8?


		project/api: REST API and a test file (how/why do we run the test)? Defines the Nation, Territory, and DiplomaticRelation classes, which are our primary database items.

		project/interactivemap: Django settings and website directory layout?


		project/interactivemap: Django settings and website directory layout?

		project: Django execution wrapper.

Conversation

wtokumaru commented Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtokumaru commented Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wtokumaru left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wtokumaru Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wtokumaru Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wtokumaru commented Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quorth0n left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wtokumaru Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

quorth0n commented Oct 7, 2018

Uh oh!

wtokumaru commented Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

wtokumaru commented Oct 7, 2018 •

edited

Loading

wtokumaru commented Oct 7, 2018 •

edited

Loading

wtokumaru Oct 7, 2018 •

edited

Loading

wtokumaru Oct 7, 2018 •

edited

Loading

wtokumaru commented Oct 7, 2018 •

edited

Loading

wtokumaru Oct 7, 2018 •

edited

Loading

wtokumaru commented Oct 7, 2018 •

edited

Loading

wtokumaru commented Oct 8, 2018 •

edited

Loading

wtokumaru commented Oct 8, 2018 •

edited

Loading

dwaxe Oct 9, 2018 •

edited

Loading