File connector and modified file messenger #2
Conversation
…mart open package and boto3 for reading in multiple keys in a bucket.
last_id of the row stored is S3
Initial implementation of feature to get last_id via notifier
cranial/messaging/file.py
Outdated
| # todo instead of write use put and pass in path | ||
| bytes_written = self.logfiles[endpoint].put( | ||
| message + '\n'.encode('utf-8'), append=append) | ||
| if bytes_written is True: |
There was a problem hiding this comment.
Connector.put just returns a boolean, so bytes_written is a misleading variable name now. (I guess it wasnt fully clear before either, unless you know that IO.write returns the number of bytes.) Maybe just call it success ?
cranial/connectors/file.py
Outdated
| return False | ||
|
|
||
| def get_last_id(self): | ||
| pass |
cranial/connectors/file.py
Outdated
| log.warning(e) | ||
|
|
||
|
|
||
| class Connector(object): |
There was a problem hiding this comment.
Should inherit from connectors.base.Connector .
There was a problem hiding this comment.
connectors.base.Connector is not functional currently because it relies on parsers which have not yet been implemented
cranial/messaging/file.py
Outdated
| """ | ||
| if sep == '': | ||
| raise Exception("Need seperator to parse last_id from file names") | ||
|
|
There was a problem hiding this comment.
The use of the id in the filename is just a convention in the pipe application. I think we just need to get the last file by date and the last line of that file and parse it, so all we have to pass in here is the bucket, path prefix, and name of the field storing the id value, which should of course default to "id".
|
|
||
| log = logger.get() | ||
|
|
||
|
|
There was a problem hiding this comment.
Why so much duplication in this file? I'm not sure we need it at all. At most, it could just be Notifier = cranial.messaging.file.Notifier, right?
There was a problem hiding this comment.
The thing is, looking through the code, cranial.messaging.file.Notifier has a lot of safeties and checks based off the os package. These checks fail for s3. I didn't want to take them out because I thought they might be useful.
|
Please merge master into you branch & resolve conflicts. |
Modified connectors.file to better get prefixes from paths with files Removed unnecessary imports Renamed bytes modified to success to make more understandable
Updated get last_id method in cranial.messaging.file
are not overwritten by defaults. Added functionality for serdes to be passed to s3 notifier Added warning for deprecated s3 connector module added closed attribute to file connector
|
Closing in favor of PR #5 |
Implemented a file connector based off the local connector to include functionality for S3 via Smart open.
Modified the file messenger to use this new file connector.