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

Double encoding of egress events #406

Open
Glutexo opened this issue Aug 20, 2019 · 1 comment
Open

Double encoding of egress events #406

Glutexo opened this issue Aug 20, 2019 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@Glutexo
Copy link
Collaborator

Glutexo commented Aug 20, 2019

The events produced on the platform.inventory.host-egress topic are currently double-encoded to JSON.

  1. First encoding happens in the app.queue.egress.build_host_event function, where Marshmallow Schema is used to validate and encode the event dictionary. This produces a JSON string.
  2. Second encoding happens in the app.queue.egress.KafkaEventProducer.write_event method by plain json.dumps.

The result is a JSON string (not a dictionary) containing properly-escaped JSON dictionary literal. "{\"type\":\"created\"}" instead of {"type":"created"}. That means that if a client decodes the message, it doesn’t get the dictionary, but a string, which must be decoded again.

Possible solutions:

  • Don’t use Marshmallow to validate messages that our code produce. These messages should be tested and thus trusted.
  • Pass the Marshmallow object instead of an encoded string. Make the producer wrapper expect such objects and dump them directly to the queue.

Passing down the Marshmallow object is an easy and quite ok solution.

I am disappoint that there are no tests for the message format. A bug like this would be probably caught by a test.

@Glutexo Glutexo assigned Glutexo and unassigned Glutexo Aug 20, 2019
@Glutexo Glutexo added the bug Something isn't working label Aug 20, 2019
@Glutexo
Copy link
Collaborator Author

Glutexo commented Sep 4, 2019

This is probably fixed by ff912a5. As far as I know, there are no tests that would confirm this though.

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
None yet
Development

No branches or pull requests

2 participants