-
Notifications
You must be signed in to change notification settings - Fork 0
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
stream-handler application #6
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. I like how the code is organized, and that it's easy to add new handlers. Thanks for the improvements and additions to the README. Your instructions look complete. I didn't try them yet, but when I do, I will note any rough spots that I find.
My comments are requests for minor changes and some additional comments.
Also, can you add a few tests? I know, I know... I didn't write any. I have no excuse. Major violation of my own rules. Tell you what... go ahead and get this ready to merge, and then I'll get some tests set up and have you review them.
|
||
|
||
## Using the Twitter APIs | ||
|
||
1. Follow the instructions at https://python-twitter.readthedocs.io/en/latest/getting_started.html to register a Twitter app. | ||
2. Under "Keys and Access Tokens," generate Consumer Key and Secret | ||
3. Copy your consumer key/secret and access key/secret into your own version of the config.py file | ||
4. Execute the following command from the root of the repo (to avoid accidentaly checking in your changes to the file) `git update-index --assume-unchanged ./config.py` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. I didn't know about that.
typo: accidentaly -> accidentally
## Using the AWS APIs | ||
|
||
1. Log-in to the AWS Console (You can create an account with your Amazon information if you haven't already. A lot of things are free for the first year. Setting up the Dynamo databases like I do below costs approximately $5 per month if you are not still on the free level) | ||
2. In the IAM section, create a user with programatic access and AmazonDynamoDBFullAccess permissions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
programatic -> programmatic
class ApplyUserFilterHandler: | ||
|
||
def __init__(self): | ||
self.log = logging.getLogger() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do getLogger(__name__)
in our modules so that we can set different levels for our code and third-party libs. I often find third-party libs way too verbose at DEBUG level.
@staticmethod | ||
def __parse_datetime(datetime): | ||
datetime = TweetType.translate_datetime(datetime) | ||
return datetime.strftime('%Y-%m-%d %H:%M:%S %Z') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd vote for using datetime.isoformat()
instead of a custom formatting string, unless there's a reason not to.
@staticmethod | ||
def __parse_datetime(datetime): | ||
_datetime = TweetType.translate_datetime(datetime) | ||
return _datetime.strftime('%Y-%m-%d %H:%M:%S %Z') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment re: datetime.isoformat()
@@ -0,0 +1,23 @@ | |||
import logging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment briefly explaining what the ApplyUserFilterHandler does and that it is intended to be called after RecordUserHandler?
def __init__(self): | ||
_dynamodb = boto3.resource('dynamodb', region_name='us-west-2') | ||
self.table = _dynamodb.Table('Users') | ||
self.log = logging.getLogger() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logging.getLogger(__name__)
from tweet_type import TweetType | ||
|
||
|
||
class WriteToconsoleHandler: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this class name should be WriteToConsoleHandler
}, | ||
ReturnValues='ALL_NEW' | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the approach to insert/update the user every time, no matter if they already exist or not? Just making sure I understand.
} | ||
) | ||
|
||
print("Table status:", table.table_status) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This schema isn't correct yet, I assume.
I forked and extended the stream-listener application. Proof of concept for the backend listener.