Skip to content

fix issue in parsing of subscribers#771

Merged
RoiTabach merged 2 commits intomasterfrom
fix-issue-770
Mar 26, 2023
Merged

fix issue in parsing of subscribers#771
RoiTabach merged 2 commits intomasterfrom
fix-issue-770

Conversation

@RoiTabach
Copy link
Copy Markdown
Contributor

we expected an empty list but subscribers can be sometimes None

we expected an empty list but subscribers can be sometimes `None`
@github-actions
Copy link
Copy Markdown
Contributor

👋 @RoiTabach
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

IDoneShaveIt
IDoneShaveIt previously approved these changes Mar 22, 2023
def list_of_lists_of_strings_to_comma_delimited_unique_strings(
list_of_strings: List[List[str]], prefix: Optional[str] = None
) -> str:
list_of_strings = [x for x in list_of_strings if x] # filter Nones and empty lists
Copy link
Copy Markdown
Contributor

@IDoneShaveIt IDoneShaveIt Mar 22, 2023

Choose a reason for hiding this comment

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

Why would we have an empty string / None / empty list in the list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Result of Alert Normalization. I expected it to be an empty list but seems it is None in some edge cases
Empty list doesn't bother us l downstream but if we're already filtering None values it makes sense to filter them as well

@IDoneShaveIt IDoneShaveIt self-requested a review March 22, 2023 11:10
@IDoneShaveIt IDoneShaveIt dismissed their stale review March 22, 2023 11:10

Added a small question

@RoiTabach RoiTabach merged commit 7f8da18 into master Mar 26, 2023
@RoiTabach RoiTabach deleted the fix-issue-770 branch March 26, 2023 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants