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

Add new keystone module and states based on shade #44713

Merged
merged 2 commits into from Dec 4, 2017
Merged

Add new keystone module and states based on shade #44713

merged 2 commits into from Dec 4, 2017

Conversation

ghost
Copy link

@ghost ghost commented Nov 28, 2017

What does this PR do?

This PR adds a new module (keystoneng.py) to supercede keystone.py. It uses the shade library from the openstack project (https://github.com/openstack-infra/shade.git) Shade is a project that has committed to maintaining the same API forever. It may add new features and more efficient ways of doing things, but it will never remove things.

Using shade will abstract the keystone v2/v3 logic away allowing the end user to write states without having to consider which version of the api is it talking to (with the exception of trying to use new features like groups/domains on v2.0 which doesn't support it).

We are making extensive use of **kwargs to help prevent the need to maintain lots of duplicate logic that exists in shade itself.
What issues does this PR fix or reference?

N/A
Tests written?

Not yet

@ghost
Copy link
Author

ghost commented Nov 28, 2017

I was asked to create a new PR for this, but here is the previous one for reference

#43207

@gtmanfred gtmanfred added Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases Awesome labels Nov 28, 2017
@gtmanfred
Copy link
Contributor

Thanks Sam!

I am adding this to my list to write tests for.

This looks great!
Daniel

@rallytime
Copy link
Contributor

@SamYaple There are some lint errors here: https://jenkins.saltstack.com/job/PR/job/salt-pr-lint-n/16717/violations/

Can you fix those?

@damon-atkins
Copy link
Contributor

damon-atkins commented Nov 29, 2017

You need to add u ( u'string') infront of every string or add
from __future__ import unicode_literals or from __future__ import absolute_import, unicode_literals

So that all strings are Unicode. As PY3 is Unicode Strings

@rallytime
Copy link
Contributor

@damon-atkins There is already work in progress to handle the unicode issues. @terminalmage Is working on it right now for Oxygen and his fixes are different than the unicode_literals import approach. Thank you very much for commenting on this, but those types of changes won't be necessary. :)

@ghost
Copy link
Author

ghost commented Nov 30, 2017

Will fix the lint issues (and gpg sign my commit)

@cachedout
Copy link
Contributor

@SamYaple Were you able to get to those lint fixes?

@damon-atkins
Copy link
Contributor

Hi @rallytime not sure how the salt team can keep retro fixing contributions, to ensure all text strings are Unicode. terminalmage job will never be done.

@rallytime rallytime merged commit aa00137 into saltstack:develop Dec 4, 2017
@gtmanfred
Copy link
Contributor

gtmanfred commented Dec 4, 2017

It's Happening

@ghost ghost deleted the keystone_shade branch December 4, 2017 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awesome Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants