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

[vdk-plugins] vdk-ingest-http: Move data conversion above size calc #1245

Merged
merged 2 commits into from
Oct 19, 2022

Conversation

doks5
Copy link
Contributor

@doks5 doks5 commented Oct 19, 2022

Currently, the memory size of the payload to be ingested is calculated before it is converted to string. This is fine for most cases, however, it gets a bit tricky for more complex objects, due to how memory size is calculated (see https://stackoverflow.com/questions/63204377/memory-size-of-list-python for reference).

This change moves the conversion of the payload to string before the memory size is calculated, so we avoid posibility to return misleading IngestionResult data.

Testing Done: Cosmetic change, all tests pass.

Signed-off-by: Andon Andonov andonova@vmware.com

Currently, the memory size of the payload to be ingested is calculated before it is
converted to string. This is fine for most cases, however, it gets a bit tricky for
more complex objects, due to how memory size is calculated (see
https://stackoverflow.com/questions/63204377/memory-size-of-list-python for reference).

This change moves the conversion of the payload to string before the memory size is calculated,
so we avoid posibility to return misleading IngestionResult data.

Testing Done: Cosmetic change, all tests pass.

Signed-off-by: Andon Andonov <andonova@vmware.com>
@doks5 doks5 force-pushed the person/andonova/ingestion-payload-size branch from bc3ceda to bb19205 Compare October 19, 2022 07:27
@doks5 doks5 merged commit cee5fc8 into main Oct 19, 2022
@doks5 doks5 deleted the person/andonova/ingestion-payload-size branch October 19, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants