Skip to content

Add --deflate option to data-pull and action-account scripts#9424

Merged
zachmargolis merged 4 commits intomainfrom
margolis-compressed-command-output
Oct 20, 2023
Merged

Add --deflate option to data-pull and action-account scripts#9424
zachmargolis merged 4 commits intomainfrom
margolis-compressed-command-output

Conversation

@zachmargolis
Copy link
Contributor

@zachmargolis zachmargolis commented Oct 20, 2023

🛠 Summary of changes

SSM commands have a max output of about 63kb, and some of our export tasks output more data than that, so they get truncated.

This adds DEFLATE compression which seems to help pack down output, so we can export more. Some notes from example testing (Slack link):

Format Size
ASCII table (current) 72,449 bytes
JSON of table data 58,721 bytes
JSON then DEFLATE 2,145 bytes (!!!!)
JSON then DEFLATE then Base64 2,908 bytes

Other notes:

  • The compression defaults to off so the calling script in devops will add it (devops PR coming soon)
  • I'm currently deploying this to our sandbox to verify that binary output is OK with SSM-command

changelog: Internal, Scripts, Add compression option to optimize script output
@zachmargolis zachmargolis requested a review from a team October 20, 2023 18:09
stdout.puts Base64.encode64(
Zlib::Deflate.deflate(
(result.json || result.table).to_json,
Zlib::BEST_COMPRESSION,
Copy link
Contributor

Choose a reason for hiding this comment

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

Did BEST_COMPRESSION make a significant difference from the defaults?

Copy link
Contributor

Choose a reason for hiding this comment

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

We also have MessagePack if we'd rather avoid the Base64 encoding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

re: BEST_COMPRESSION, I didn't try others
re: messagepack, I will give it a shot! I thought it was a binary format for some reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just confirmed that message pack is binary, I think it'll run into the same issue that the binary DEFLATE data had, which is that SSM command seems to eat it.

The original reason I chose JSON was this blog post that seemed to indicate JSON + GZIP yielded smaller payloads than MSGPACK

Copy link
Contributor

Choose a reason for hiding this comment

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

ah got it, sounds good!

Copy link
Contributor

@n1zyy n1zyy left a comment

Choose a reason for hiding this comment

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

Nice! This compresses so much better than I would have expected.

@zachmargolis zachmargolis merged commit 5d1d94e into main Oct 20, 2023
@zachmargolis zachmargolis deleted the margolis-compressed-command-output branch October 20, 2023 22:11
@mdiarra3 mdiarra3 mentioned this pull request Oct 23, 2023
mdiarra3 added a commit that referenced this pull request Oct 24, 2023
* LG-11083: Enable USPS Public Endpoint (#9355)

* changelog: Internal, In-Person Proofing, Enable public USPS post office search

* Use EnrollmentHelper to switch between mock/real thing

* Try behaves_like

* Revert shared examples for now

* Use full name

* Update report mailer preview to be more realistic (#9419)


**How**: stubs CloudwatchClient

changelog: Internal, Reporting, Updates report preview to use live code

* Add analytics section to frontend documentation (#9421)

* Add analytics section to frontend documentation

changelog: Internal, Documentation, Add analytics frontend documentation

* link to correct javascript package

* LG-11101: Support multiple valid MFA to satisfy authentication request (#9335)

changelog: User-Facing Improvements, MFA, Avoid prompting for MFA in some scenarios where a recent MFA satisfies the requirement

* LG-11148 | Adds monthly report on total verified users (#9376)

changelog: Internal, Reporting, Monthly report now includes total verified users

Also incorporates LG-11150

Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>

* Remove second MFA prompt exception for strict MFA requirement (#9422)

changelog: User-Facing Improvements, MFA Setup, Add second MFA reminder screen for single-MFA accounts when signing in at AAL2

* LG-11126 Update Start over verifying your identity screen (#9313)

* change text for start over verify screen

* add translations for page

* add changelog

changelog: User-Facing Improvements, IdV By Mail, update text in start
over verifying identity screen

* remove unused i18n

* create new translation with question mark added

* current step indicator for user not in gpo flow yet

* a missing period

* Restore deleted translations, and rename start_over to start_over_new_address

Co-authored-by: Doug Price <douglas.price@gsa.gov>

* New template for confirm start over from request_letter

Add source param to indicate whether referer is request_letter

* Update specs to check for correct template

Co-authored-by: Doug Price <douglas.price@gsa.gov>

* Add before_letter route for new screen, don't use it yet

And analytics

* Lint, unused arg in analytics_events

* alphabetization lint

* Add suggested comment

Co-authored-by: Matt Hinz <matt.hinz@gsa.gov>

* lints

---------

Co-authored-by: Douglas Price <douglas.price@gsa.gov>
Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
Co-authored-by: Matt Hinz <matt.hinz@gsa.gov>

* LG-11198: Update address text (#9420)

Update address text

changelog: User-Facing Improvements, IdV, Update text for address

* LG-10922: Display new headings for Hybrid Handoff page on AB test (#9316)

* changelog: User-Facing Improvements, Doc Auth, Display new headings for Hybrid Handoff page on AB test

Adds:

* Conditional headers depending on which flag is on
* Hybrid handoff show view test
* Translations

* LG-11235: Rename double address verification as ipp_enrollment_in_progress (#9390)

* Removed double address verification replaced with ipp_enrollment_in_progress

* changelog: Internal, In-person Proofing, change DAV references to reflect reality

* Change test description to be closer to what is being changed in the controller

* Addressing 50/50 state concerns in proofer and adjudicator

* Addressing linter issues

* Set missing initial value for dav

* Moving arg with default value to end of list

* Apply suggestions from code review

Adding proper input to job_arguments hash.

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Adding note about existing ticket for work post 50/50 state

* Resolving Shannon's comments

* Adding back in test for dav, need reader on adjudicator

* Adding back in test for dav, need reader on adjudicator

---------

Co-authored-by: jack.ryan@gsa.gov <johnaryan@fcoh2j-f4t79kf4.myfiosgateway.com>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Add --deflate option to data-pull and action-account scripts (#9424)


changelog: Internal, Scripts, Add --deflate option to data-pull and action-account scripts

---------

Co-authored-by: Matt Gardner <wilburnforce@gmail.com>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Matt Wagner <mattwagner@navapbc.com>
Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>
Co-authored-by: Alex Bradley <alexander.bradley@gsa.gov>
Co-authored-by: Douglas Price <douglas.price@gsa.gov>
Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
Co-authored-by: Matt Hinz <matt.hinz@gsa.gov>
Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com>
Co-authored-by: Brittany Greaner <35475380+night-jellyfish@users.noreply.github.com>
Co-authored-by: Jack Ryan <jackryan@navapbc.com>
Co-authored-by: jack.ryan@gsa.gov <johnaryan@fcoh2j-f4t79kf4.myfiosgateway.com>
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