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

Accomodate large command results #26

Merged
merged 5 commits into from
Aug 10, 2021
Merged

Accomodate large command results #26

merged 5 commits into from
Aug 10, 2021

Conversation

w0de
Copy link
Contributor

@w0de w0de commented Aug 5, 2021

Hello!

When handling the command result response for InstalledApplicationList, nanomdm issues the following plea:

level=info handler=checkin-command msg=command report results err=storing command report: Error 1406: Data too long for column 'result' at row 1

The InstalledApplicationList response for my current device - with only base Apple apps - is 117,465 characters of base64-endcoded text. TEXT support 65,525 characters; MEDIUMTEXT, 16,777,215 (this corresponds to ~16.78 mb: "L + 3 bytes, where L < 2^24" - https://dev.mysql.com/doc/refman/8.0/en/storage-requirements.html#data-types-storage-reqs-strings)

This works with current nanomdm release (in as much as I migrated with schema.00005.sql and everything continued to work/InstalledApplicationList started working). MySQL docs do not have any warnings regarding migrating between text types that I found.

@jessepeterson
Copy link
Member

Good catch, thanks! We'll also need to have a schema change file to do an ALTER TABLE.

@jessepeterson
Copy link
Member

Also: we could go larger with LONGTEXT as well.

@w0de
Copy link
Contributor Author

w0de commented Aug 5, 2021

Also: we could go larger with LONGTEXT as well.

I thought about it but proposed MEDIUMTEXT since LONGTEXT can be up to 4GB and 4,294,967,295 - I didn't know if setting someone's DB up for that sort of storage requirement was too big an assumption. Will change if you'd prefer LONGTEXT

@w0de
Copy link
Contributor Author

w0de commented Aug 5, 2021

Good catch, thanks! We'll also need to have a schema change file to do an ALTER TABLE.

I added schema.00005.sql with the ALTER statement, is that what you mean?

storage/mysql/schema.00005.sql Outdated Show resolved Hide resolved
Co-authored-by: Jesse Peterson <jessepeterson@users.noreply.github.com>
@w0de w0de requested a review from jessepeterson August 9, 2021 16:13
@jessepeterson jessepeterson merged commit 7a3c035 into micromdm:main Aug 10, 2021
w0de added a commit to w0de/nanomdm that referenced this pull request Dec 14, 2021
* change command_result.result to `MEDIUMTEXT`
* create storage/mysql/schema.00005.sql
w0de added a commit to w0de/nanomdm that referenced this pull request Mar 17, 2022
* change command_result.result to `MEDIUMTEXT`
* create storage/mysql/schema.00005.sql
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.

2 participants