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

Restore IPA 3.0 compatibility of copy-schema-to-ca.py #372

Closed
wants to merge 1 commit into from

Conversation

tiran
Copy link
Member

@tiran tiran commented Jan 5, 2017

Apparently ipaplatform.paths is not available on IPA 3.x.

https://fedorahosted.org/freeipa/ticket/6540

Signed-off-by: Christian Heimes cheimes@redhat.com

Apparently ipaplatform.paths is not available on IPA 3.x.

https://fedorahosted.org/freeipa/ticket/6540

Signed-off-by: Christian Heimes <cheimes@redhat.com>
@stlaz
Copy link
Contributor

stlaz commented Jan 5, 2017

Is there a reason not to stick with the original ipautil.SHARE_DIR and without setting confdir? This script won't be run on servers that either need confdir set or have ipaplatform.paths, will it (I know I acked the latter, did not realize there would be trouble)?

@tiran
Copy link
Member Author

tiran commented Jan 5, 2017

SHARE_DIR is no longer available. I had to find another approach. The approach import else use well-known constants is safe and will not break any time soon.

@MartinBasti
Copy link
Contributor

this script must work only with IPA3.x, so I wouldn't add there anything from 4.4/master code. As I pointed out several times I don't think that this code even should be in master branch, as we are always just fixing regressions there, but rather as separate script, or script in IPA3.x. Unfortunately I haven't got enough support for my idea.

@HonzaCholasta
Copy link
Contributor

I agree with @mbasti-rh. IMO we should remove all 4.0+ specific code from the script, add a version check at the beginning and disable all failing pylint checks. Maybe also add a comment at the top saying that the file is not to be modified anymore.

@stlaz
Copy link
Contributor

stlaz commented Jan 17, 2017

+1, that was actually my original point. Just revert the change done to the file in https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=d6b755e3fcaf32158f4ee36d45e3344b4a03fbc2, don't add confdir option to api.bootstrap() and let the script die in peace.

@tiran
Copy link
Member Author

tiran commented Jan 17, 2017

How about we remove the file entirely and just post it on the wiki or something?

@martbab
Copy link
Contributor

martbab commented Jan 17, 2017

IIRC @mbasti-rh proposed to maintain the script separately and serve it to users via external repo or something but the idea was rejected.

@MartinBasti
Copy link
Contributor

I proposed 2 ideas:

  • move it to ipa-3-3 branch (or)
  • extract that script from freeipa repo and allow to download that script from freeipa.org (and access.redhat.com)

@tiran
Copy link
Member Author

tiran commented Jan 17, 2017

So with separate script you meant a separate downloadable version of the script. Got it! :)

It seems we have a consent. @mbasti-rh I second your proposal to move it to freeipa.org (that what I meant with wiki) and access.redhat.com.

@stlaz
Copy link
Contributor

stlaz commented Jan 17, 2017

+1, we need to fix the script first, though.

@tiran
Copy link
Member Author

tiran commented Jan 17, 2017

Or we just grab a working and tested version from an old release.

@MartinBasti
Copy link
Contributor

@tiran +1, but first this has to be generally approved :) topic for meeting maybe (today?)

@tiran
Copy link
Member Author

tiran commented Jan 18, 2017

I have updated the ticket https://fedorahosted.org/freeipa/ticket/6540#comment:5 with the result of this discussion. I'm going to close the PR. Let's start a new one to remove it and update ipaserver/install/cainstance.py plus builds.

@tiran tiran closed this Jan 18, 2017
@tiran tiran deleted the copyschemaca_fix branch January 18, 2017 08:53
@stlaz stlaz added the rejected Pull Request has been rejected label Jan 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rejected Pull Request has been rejected
Projects
None yet
5 participants