-
Notifications
You must be signed in to change notification settings - Fork 394
Add a prefix to tags cache file names #751
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
Conversation
1d7a85b to
f075d18
Compare
Use lambda function name as a prefix when creating tags cache files accessed by the forwarder. This will allow to use the same S3 bucket to store cache files from different forwarderst push
f075d18 to
1275418
Compare
|
|
||
|
|
||
| class BaseTagsCache(object): | ||
| CACHE_PREFIX = None |
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 we use lowercased, instance variables, initialized by the constructor for these 3 ?
The way this is defined here makes it possible to create an instance of BaseTagsCache (or subclass) which does not properly initialize the cache prefix
| parsed = parse(event, context) | ||
| enriched = enrich(parsed) | ||
| # set the prefix for cache layer | ||
| if not cache_layer.prefix: |
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 we instead create the CacheLayer with the proper prefix directly here ? and pass the prefix to Caches in CacheLayer.__init__
| """Retrieves extra tags from lambda, either read from the function arn, or by fetching lambda tags from the function itself. | ||
| Args: | ||
| log (dict<str, str | dict | int>): a log parsed from the event in the split method |
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 best arg name log or log_event ?
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.
It will be a lambda log event here given the context. I think it should be good to use log_event
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.
OK, please just rename it in the comment then.
Co-authored-by: Michel Daviot <michel@daviot.info>
a0166a7 to
988b073
Compare
What does this PR do?
Add a prefix string to tags cache filenames using a hash of the function ARN
This will allow to store cache files on a single S3 bucket for multiple Lambda forwarders.
We can access the function ARN from the context object provided by AWS on function invocation
https://docs.aws.amazon.com/lambda/latest/dg/python-context.html
Also, update readme file by adding a troubleshooting section when encountering issues creating S3 triggers
Motivation
Testing Guidelines
Additional Notes
Types of changes
Check all that apply