Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

bug: doc fixup #1261

Merged
merged 1 commit into from
Jun 21, 2018
Merged

bug: doc fixup #1261

merged 1 commit into from
Jun 21, 2018

Conversation

jrconlin
Copy link
Member

  • Fix out of date documentation
  • Add AWS_LOCAL_CREDS to pass local credential info (thanks boto3)
  • Fix tests to use new local env var (like we do for endpoint)
  • Add rust migration document
  • Fix autokey to return something useful
  • Point out that docker_compose does bad things to your expectations
    about CRYPTO_KEY

Closes #1260
Issue mozilla-services/autopush-rs#7

@codecov-io
Copy link

codecov-io commented Jun 19, 2018

Codecov Report

Merging #1261 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1261   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          59      59           
  Lines        9523    9525    +2     
======================================
+ Hits         9523    9525    +2
Impacted Files Coverage Δ
autopush/tests/__init__.py 100% <100%> (ø) ⬆️
autopush/config.py 100% <100%> (ø) ⬆️
autopush/tests/test_db.py 100% <100%> (ø) ⬆️
autopush/db.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3babedc...b8c3201. Read the comment docs.

autopush/db.py Outdated
@@ -468,6 +468,10 @@ def __init__(self, **kwargs):
conf = kwargs
if not conf.get("endpoint_url"):
conf["endpoint_url"] = os.getenv("AWS_LOCAL_DYNAMODB")
creds = os.getenv("AWS_LOCAL_CREDS")
Copy link
Member

Choose a reason for hiding this comment

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

autopush-rs hardcodes creds to bogus whenever AWS_LOCAL_DYNAMODB is set. I'm thinking we do the same here (killing AWS_LOCAL_CREDS), as I don't think anyone will ever need custom creds when pointing to a local ddb

Copy link
Member Author

Choose a reason for hiding this comment

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

I was debating that myself. The only potential reason I could come up with to keep them separate was the low percentile chance that they actually had some specific credentials that they wanted to use.

Yeah, I have really dumb arguments with myself.

pjenvey
pjenvey previously approved these changes Jun 19, 2018
hostname: autopush
environment:
- LOCAL_HOSTNAME=localhost
- ROUTER_HOSTNAME=autopush
- AWS_LOCAL_DYNAMODB=http://dynamodb:8000
- AWS_LOCAL_CREDS=Bogus:Bogus
Copy link
Member

Choose a reason for hiding this comment

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

one more thing, forgot to kill it here and below (possibly in the commit message too?)

@jrconlin jrconlin force-pushed the docs branch 2 times, most recently from a3becaa to fdb3811 Compare June 20, 2018 20:26
pjenvey
pjenvey previously approved these changes Jun 20, 2018
@@ -1,7 +1,8 @@
version: '2'
services:
autopush:
image: bbangert/autopush
# image: bbangert/autopush
Copy link
Member

Choose a reason for hiding this comment

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

Just delete this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was actually debating if we wanted to make this use the image we generate when we tag a release, rather than doing a local build.

docs/index.rst Outdated
@@ -10,6 +10,11 @@ autopush

Mozilla Push server and Push Endpoint utilizing PyPy, twisted, rust, and DynamoDB.
Copy link
Member

Choose a reason for hiding this comment

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

Strike rust.

bbangert
bbangert previously approved these changes Jun 20, 2018
* Fix out of date documentation
* Fix tests to use new local env var (like we do for endpoint)
* Add rust migration document
* Fix autokey to return something useful
* Point out that docker_compose does bad things to your expectations
  about CRYPTO_KEY

Closes #1260
Issue mozilla-services/autopush-rs#7
# the previous 'build:' line and uncomment the following
# 'image:' line. This will use the latest production
# built image.
#image: mozilla/autopush
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll note that the current mozilla/autopush image doesn't work with this version of docker-compose, thus the comment for now. Obviously, the most stable thing is to build locally, but if we want folks to just play, we can switch the defaults here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants