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

Fix 57133 - bytes vs text encoding issue for sqlite sdb #61794

Merged
merged 7 commits into from
Apr 7, 2022

Conversation

waynew
Copy link
Contributor

@waynew waynew commented Mar 15, 2022

What does this PR do?

What issues does this PR fix or reference?

Fixes: #57133

Previous Behavior

When using SQLite as the sdb backend, a string would not survive a round-trip:

salt-call sdb.set 'sdb://example_profile/key' somevalue
salt-call sdb.get 'sdb://example_profile/key' --out=json
"b'somevalue'"

Oops!

New Behavior

Now when calling msg.unpackb we tell it raw=False - strings should be strings,
an bytes should be bytes. The test parameters verify that bytes still come back
as bytes, text comes back as texts, and other assorted values correctly survive
the round trip.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

- [ ] Docs

Commits signed with GPG?

Yes

Right now sqlite sdb.get will fail for most purposes because the text
values are not decoded. This exhibits that behavior (except of course
for the lone byte-string parameter).
@waynew waynew requested a review from a team as a code owner March 15, 2022 22:23
@waynew waynew requested review from twangboy and removed request for a team March 15, 2022 22:23
Ch3LL
Ch3LL previously approved these changes Mar 16, 2022
@Ch3LL Ch3LL added the Phosphorus v3005.0 Release code name and version label Mar 16, 2022
@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 16, 2022

The windows test failures are related to this change.

Before it was pinned to an out of date buggy version, this should fix
some errors and inconsistencies.
@@ -6,3 +6,4 @@ requests>=1.0.0
distro>=1.0.1
contextvars
psutil>=5.0.0

Copy link
Contributor

Choose a reason for hiding this comment

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

remove empty line

@waynew waynew self-assigned this Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phosphorus v3005.0 Release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]sdb:// entries in salt-cloud providers files not being resolved
2 participants