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

Don't return bytes if py3 in salt-cloud #34105

Merged
merged 3 commits into from
Jun 21, 2016

Conversation

cachedout
Copy link
Contributor

What does this PR do?

We used to return bytes instead of unicode, which would make a dict with mixed bytes and unicode. This was a pain and caused tests to fail.

What issues does this PR fix or reference?

Failing py3 compat in salt-cloud

Previous Behavior

A dict that looked like ..{'default': {'disk': 0, 'id': b'1', 'driver': <libcloud.compute.drivers.dimensiondata.DimensionDataNodeDriver object at 0x7f2fcbc769b0>, 'price': 0, 'get_uuid': <bound method UuidMixin.get_uuid of <NodeSize: id=1, name=default, ram=0 disk=0 bandwidth=0 price=0 driver=DimensionData ...>>, 'ram': 0, 'name': b'default', 'uuid': b'94811c79b526d5ef99518ce215d56123cc644067', 'bandwidth': 0, 'extra': {}}} for the test return in unit.cloud.clouds.dimensiondata_test.DimensionDataTestCase.test_avail_sizes

cc: @techhat @rallytime and @s0undt3ch

@s0undt3ch This affects changes that you made specifically, so please have a quick look if you can.

@rallytime
Copy link
Contributor

@cachedout Can you clean up those pylint errors?

@cachedout
Copy link
Contributor Author

@rallytime Thanks. Fixed.

'ascii', 'salt-cloud-force-ascii'
)
if not six.PY3:
vm_image = config.get_cloud_config_value('image', vm_, __opts__).encode(
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we still need a vm_image set if we are running python 3? Similar to the change in line 246 above: https://github.com/saltstack/salt/pull/34105/files#diff-e8c33e1c4a43de8e11379a6d5cf064b0R246

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gah. Yeah. Sorry. Thanks.

@rallytime rallytime merged commit 2efcdad into saltstack:develop Jun 21, 2016
gitebra pushed a commit to gitebra/salt that referenced this pull request Jun 22, 2016
* upstream/develop: (25 commits)
  Support using private IPs (without pubic IPs) (saltstack#34187)
  Add 'delete' key event to list (saltstack#34189)
  Don't return bytes if py3 in salt-cloud (saltstack#34105)
  Raise the correct exception when gitfs lockfile is empty
  Remove unnecesssary comment
  Fix diskusage beacon
  fix salt --summary to count not responding minions correctly (saltstack#34165)
  doc: add missing dot (saltstack#34175)
  Typo fix (saltstack#34174)
  Update docs to match log_level warning default
  Fix YAML indentation in Apache state docstrings (saltstack#34166)
  Add integration tests for grains.append
  Moved imports to top, out of _get_moto_version function
  Updated version check. Moved check into it's own function
  Move log message from INFO to DEBUG.
  Updated test to work with new moto version. Changed strings to unicode
  Update package dep note to systemd-python for RHEL7 install
  _active_mounts_openbsd: unbreak output for special filesystems
  Make docker.absent honor test=true
  When specifying the SSH identity to use with Git as a salt URL, eg. salt://files/identity, if that file exists outside of the default base environment the file won't be accessible so we need to include the saltenv.
  ...

# Conflicts:
#	salt/states/git.py
@s0undt3ch
Copy link
Collaborator

Looked. 👍

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.

None yet

3 participants