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

Support headers #208

Merged
merged 8 commits into from
Jun 4, 2024
Merged

Support headers #208

merged 8 commits into from
Jun 4, 2024

Conversation

clyang82
Copy link
Contributor

fixed: #207

Signed-off-by: clyang82 <[email protected]>

Test

Signed-off-by: clyang82 <[email protected]>
Copy link
Contributor

@Alex-Izquierdo Alex-Izquierdo left a comment

Choose a reason for hiding this comment

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

Hi @clyang82 Thanks for your contribution.

I think there is no need to make it dependent on an option in the plugin and we could just send the headers always. What do you think @bzwei?

Tests should be updated to check that we send correctly the headers in the event.

@clyang82
Copy link
Contributor Author

I am afraid sending the headers always may break the backward compatibility if someone is dealing with the whole event, including body and meta.

@Alex-Izquierdo
Copy link
Contributor

Alex-Izquierdo commented May 21, 2024

I am afraid sending the headers always may break the backward compatibility if someone is dealing with the whole event, including body and meta.

I think there is no risk at all, because we are extending the existing schema of the event with a new root key of the event, preserving the existing "body" key. To me we should send the headers always.

cc @bzwei @mkanoor

Signed-off-by: clyang82 <[email protected]>
@clyang82
Copy link
Contributor Author

I am afraid sending the headers always may break the backward compatibility if someone is dealing with the whole event, including body and meta.

I think there is no risk at all, because we are extending the existing schema of the event with a new root key of the event, preserving the existing "body" key. To me this is a bug and we should send the headers always.

cc @bzwei @mkanoor

Thanks for your comments. Remove the option to send the headers always.

Alex-Izquierdo
Alex-Izquierdo previously approved these changes May 21, 2024
@Alex-Izquierdo Alex-Izquierdo requested review from bzwei and mkanoor May 21, 2024 15:16
Alex-Izquierdo
Alex-Izquierdo previously approved these changes May 30, 2024
@Alex-Izquierdo Alex-Izquierdo merged commit 1084732 into ansible:main Jun 4, 2024
7 checks passed
@ssbarnea ssbarnea added the bug Something isn't working label Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Include headers in kafka message
4 participants