Add string sets to helpers #55
Conversation
global_helpers/panther.py
Outdated
|
|
||
|
|
||
| def reset_counter(key: str) -> None: | ||
| """Reset a counter to 0.""" | ||
| kv_table().put_item(Item={'key': key, _COUNT_COL: 0}) | ||
|
|
||
|
|
||
| def set_counter_expiration(key: str, epoch_seconds: int) -> None: | ||
| """Configure the counter to automatically expire at the given time. | ||
| def set_key_expiration(key: str, epoch_seconds: int) -> None: |
There was a problem hiding this comment.
Works for both string sets and counters (which can coexist under the same key), renaming for generality
global_helpers/panther.py
Outdated
|
|
||
|
|
||
| def remove_from_string_set(key: str, val: Union[str, | ||
| Sequence[str]]) -> Set[str]: |
There was a problem hiding this comment.
yapf made me do it this way, though I strongly disagree :P
Makefile
Outdated
| @@ -13,7 +13,7 @@ deps-update: | |||
|
|
|||
| lint: | |||
| yapf $(analysis_directories) --diff --parallel --recursive --style google | |||
| bandit -r $(analysis_directories) | |||
| bandit -r $(analysis_directories) --skip B101 | |||
There was a problem hiding this comment.
What check is B101? Could you add a comment explaining why/what is being skipped, or use a long form name if that's possible (like in pylint)?
There was a problem hiding this comment.
Long-form isn't possible with bandit, but I can add a comment. It's just warning about the "assert" statements, which are ignored when compiling Python into bytecode
global_helpers/panther.py
Outdated
| assert get_string_set('panther') == set() | ||
|
|
||
|
|
||
| if __name__ == '__main__': |
There was a problem hiding this comment.
Can we include this and the tests in a separate file? IIRC this file is shown in the UI cc @nhakmiller
There was a problem hiding this comment.
After my most recent change this is no longer shown in the UI, it is saved as panther_oss_helpers and only accessible from the CLI (until if/when we add support for multiple globals in the UI).
There was a problem hiding this comment.
I'm happy to move this somewhere else in the future, I just wanted to quickly save it somewhere. I wasn't sure how best to do an "integration test" in this repo
|
|
||
|
|
||
| def reset_counter(key: str) -> None: | ||
| """Reset a counter to 0.""" | ||
| kv_table().put_item(Item={'key': key, _COUNT_COL: 0}) | ||
|
|
||
|
|
||
| def set_counter_expiration(key: str, epoch_seconds: int) -> None: | ||
| """Configure the counter to automatically expire at the given time. | ||
| def set_key_expiration(key: str, epoch_seconds: int) -> None: |
There was a problem hiding this comment.
nit: Thinking maybe better to rename to set_approx(imate)_key_expiration? The description clarifies that items will be deleted within 48hrs of expiration time, but thinking have it in the name will make it more obvious (few people read documentation :P )
There was a problem hiding this comment.
I know what you mean, but approx_expiration_time to me suggests the actual value saved to Dynamo may not be what you passed to the function. The expiration time is very precise and written to the table exactly as specified and the item is then guaranteed to expire. It's just the system may not honor it immediately
Typically, from what I've seen so far, the expiration happens within a few minutes. I think it's reasonable to assume people will not expect a distributed system to respond immediately
Co-authored-by: Evan Gibler <evan.gibler@panther.com>
#1076) Co-authored-by: akozlovets098 <95437895+akozlovets098@users.noreply.github.com>
Background
Follow- up to #54 which adds string sets as well.
Changes
put_string_set- overwrite the entire set at onceadd_to_string_set- add one or more elements to a setremove_from_string_set- remove one or more elements from a setreset_string_set- remove all entries in a setTesting
make fmt test lintI added the test cases from #54 as well as the ones I used for string sets as some simple assert statements. I don't think we need the full unit testing library here, but I do want to keep the testing code around so (a) we can test changes and (b) users can see how we the functions are intended to be used