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

S3 pillar cache file should be opened 'wb' on python3 #54698

Closed
wants to merge 1 commit into from

Conversation

rklaren
Copy link
Contributor

@rklaren rklaren commented Sep 20, 2019

What does this PR do?

This fixes the S3 pillar on python3. In python3 the file that is written to with pickle.dump needs to be opened in binary write mode. The fix should be backwards compatible to python2 AFAIK.

What issues does this PR fix or reference?

Fixes #54696

Previous Behavior

Previously fetching the pillar data would fail on python3 due to the writing of the cache throwing an exception.

New Behavior

It works.

Tests written?

No. I'd not mind giving a stab at it if someone can point me at test fixtures for EC2.

Commits signed with GPG?

Yes.

@rklaren rklaren requested a review from a team as a code owner September 20, 2019 03:01
@ghost ghost requested a review from dwoz September 20, 2019 03:01
@waynew
Copy link
Contributor

waynew commented Oct 1, 2019

@rklaren there are some tests in tests/integration/cloud/clouds/test_ec2.py. I'm not familiar with that area, looks like @Ch3LL and @dwoz have been involved in that file, so they may have better input there.

On a different note - we're planning to change our branch process, so if you could rebase your change on the master branch and edit this PR to point to master (click the "edit" button at the top of the PR), that would be great!

@rklaren rklaren changed the base branch from 2019.2 to master October 10, 2019 01:11
@rklaren rklaren changed the base branch from master to 2019.2 October 10, 2019 01:14
@rklaren
Copy link
Contributor Author

rklaren commented Oct 10, 2019

Thanks! I'll have a look at both rebasing and the those tests.

@dwoz
Copy link
Contributor

dwoz commented Dec 9, 2019

@rklaren This needs to be re-opened against the master branch. We are no
longer accepting PRs to the 2019.2. If this fixes a bug in a 2019.2.x
release, please make note of it in your PR to master. Thanks!

@dwoz dwoz closed this Dec 9, 2019
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