Skip to content
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

Handling Added events #212

Merged
merged 2 commits into from
Jan 25, 2024
Merged

Conversation

EmanElsaban
Copy link
Contributor

@EmanElsaban EmanElsaban commented Jan 19, 2024

This PR currently contains:

  • handling of Added events

@EmanElsaban EmanElsaban changed the title Handling Added events and adding timers in signalfx and logs in scribereader Handling Added events and logs in scribereader Jan 19, 2024
@EmanElsaban EmanElsaban force-pushed the u/emanelsabban/handling-added-events branch from 4e03b9b to 52b20a5 Compare January 19, 2024 21:43
Copy link
Member

@nemacysts nemacysts left a comment

Choose a reason for hiding this comment

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

(we also chatted a bit internally)

@@ -388,7 +471,7 @@ def _process_pod_event(self, event: PodEvent) -> None:
elif event["type"] == "DELETED":
self.__handle_deleted_pod_event(event)

elif event["type"] == "MODIFIED":
elif event["type"] == "MODIFIED" or event["type"] == "ADDED":
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
elif event["type"] == "MODIFIED" or event["type"] == "ADDED":
elif event["type"] in {"MODIFIED", "ADDED"}:

would be a little more idiomatic/shorter, but the current code works as well :)

@EmanElsaban EmanElsaban force-pushed the u/emanelsabban/handling-added-events branch 4 times, most recently from 1ee611d to 1c84fd1 Compare January 19, 2024 21:56
Comment on lines 407 to 410
# we might see that their are gaps 0.5s because thats how long it will take it to see if there are stuff in the queue
# will give you whats in the queue or wait 0.5 sec to receive events, if no events are received then it will throw the empty exception
# and start again
# I think below might be taking some time to get from a queue an event, should time these two separately
Copy link
Member

Choose a reason for hiding this comment

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

we can probably remove this since there's no more timers here :)

@@ -210,7 +210,7 @@ def __update_modified_pod(self, pod: V1Pod, event: Optional[PodEvent]) -> None:

if pod.status.phase not in SUPPORTED_POD_MODIFIED_EVENT_PHASES:
logger.debug(
f"Got a MODIFIED event for {pod_name} for unhandled phase: "
f"Got a {event['type']} event for {pod_name} for unhandled phase: "
Copy link
Member

Choose a reason for hiding this comment

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

i wonder if we can do something like:

    def __update_modified_pod(self, pod: V1Pod, event: Optional[PodEvent]) -> None:
        ...
        event_type = event['type'] if event else "null"
        ...

to handle the null event case (from reconciling)

@EmanElsaban EmanElsaban force-pushed the u/emanelsabban/handling-added-events branch from 1c84fd1 to 0beaaac Compare January 19, 2024 22:35
@nemacysts nemacysts changed the title Handling Added events and logs in scribereader Handling Added events Jan 23, 2024
@EmanElsaban EmanElsaban merged commit 80d4980 into master Jan 25, 2024
2 checks passed
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.

3 participants