search

DistrictDataLabs / cultivar

Multidimensional data explorer and visualization tool.
http://trinket.districtdatalabs.com
Apache License 2.0
52 stars 18 forks source link

added a check to the base.py settings file to see if AWS settings are defined in env file #89

Closed bahadasx closed 7 years ago

bahadasx commented 7 years ago

Addresses: #85

The changes I made include the following:

@looselycoupled Can you please check over this PR when you get a chance? If you don't have time, I can try to punt it over to @tojeda instead. Thanks! :smile:

bahadasx commented 7 years ago

The first time TravisCI ran, it failed because of a social_django module. I did some googling to try and figure it out and did a new push to the PR. Travis ran again and this time failed, but there was no error log. Not sure why the make test is failing here?

bahadasx commented 7 years ago

@looselycoupled @ojedatony1616 So, the social auth dependencies have been driving me nuts. First, the make test wasn't working due to dependencies and set up. After I fixed that, then, I noticed running the app via python manage.py runserver 0.0.0.0:8000 produced a map has no len error. So, I had to play around with the dependencies again (the issue was with open_id). So, after I felt like I fixed it, I removed my virtualenv, created a new one, and installed requirements from scratch. Then I ran the make test and ran the main server again and everything appears to be working fine now. I just wanted to briefly summarize what occurred so you can understand why I changed versions of certain dependencies. Please take a look at this PR when you get a chance. Thanks 😄

bahadasx commented 7 years ago

@looselycoupled @ojedatony1616 I will add one more thing to note. As noted in the base.py file, when the DEFAULT_FILE_STORAGE parameter is using the default setting, it uses the MEDIA_ROOT setting to define the location in which files are stored. I noticed this is defined in the development.py file: MEDIA_ROOT = os.path.join(PROJECT, 'media') but not in the production.py file. I'm not sure if it makes sense to add this to the prod file, but I thought I would note it here.

looselycoupled commented 7 years ago

Thanks @bahadasx, and yes the social auth stuff is known to be a problem right now and I think there is an open ticket for it. I definitely appreciate the work you've put into this. I'll take a look when I can but it may not be until Sunday.

lauralorenz commented 7 years ago

Whaaaat a can of worms that was. I was going down the 'check if OAuth still works' rabbit hole for a while there. Basically turns out out that the package we are using deprecated last December (see deprecation note) and the way they decided to do that was to bump up to version 0.3+ that basically turned it into a skin of the new package which is configured differently (and breaks those configured against the 0.2.x series). So maybe we want to migrate to that for real some time, but I'm on board with pinning to the latest 0.2.x series for now (I put it to the latest/final non-deprecated version 0.2.19).

I added some docs and took out some unnecessarily restricting pip requirements since it worked with the latest on those and I'm not sure how pin-happy we want to be as a general rule.

@bahadasx I would say it's fine to leave the MEDIA_ROOT setting out of production.py since we expect to have AWS credentials set in production every time.

Gonna go ahead and merge this and ask for permission/forgiveness laterrrr @looselycoupled fight me

bahadasx commented 7 years ago

@lauralorenz Thanks for looking over this. Yes, it was a whole can of worms to deal with!