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

Python2->Python3 update, Python2 unsafe #20

Closed
wants to merge 7 commits into from

Conversation

qinzhang22
Copy link

@qinzhang22 qinzhang22 commented Nov 13, 2023

For python-pam, this is already built with xs8 and xs9. The XS8 package is in rt-next now.

xsconsole xenrt test passed: 3864890

@qinzhang22 qinzhang22 force-pushed the pr15-split-part5 branch 2 times, most recently from bf3398f to 3f8490f Compare November 13, 2023 05:53
@liulinC
Copy link
Collaborator

liulinC commented Nov 13, 2023

Do not forget to tag before this merge, with major version bump up (for in-compatibility)

@liulinC liulinC changed the title Pr15 split part5 Python2->Python3 update, Python2 unsafe Nov 21, 2023
@@ -226,7 +226,7 @@ def UpdateFieldsCONFIRM(self):
pane.ResetFields()

sr = HotAccessor().sr[self.srHandle]
srName = sr.name_label(None).encode('utf-8')
srName = Lang(sr.name_label(None))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why've you used Lang() here and elsewhere in the commit? I think it is not normally used with dynamic values. Normally it is only used with static labels like Operation, Storage Repository, etc.

Copy link
Author

Choose a reason for hiding this comment

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

I used Lang() as I'm not sure the object is a string or a bytes object. Let's remove encode here to see how it goes.

Copy link
Collaborator

@bernhardkaindl bernhardkaindl left a comment

Choose a reason for hiding this comment

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

Please apply change request that I originally made in PR 18 to keep Python3 working:
#18 (comment)

Here is Pau's hard disagree comment again:
#15 (comment)
Andrew's initiative to keep Python2
#15 (comment)
and also Alex questioned splitting the code base of packages recently when there is no need to do so.

This change is small and even reduces the amount of changed lines:

try:
    import xmlrpclib
    import SocketServer
    import SimpleXMLRPCServer
except:
    import xmlrpc.client as xmlrpclib
    import socketserver as SocketServer
    import xmlrpc.server as SimpleXMLRPCServer

In addition, there are xsconsole plugins on xs8. You can find one with rpm -qa on xs8

rpm -qa | grep xsconsole

What about xsconsole plugins by partners?
Can we check them?

Until these are checked and updated as well, we cannot declare xsconsole update work as finished.

@qinzhang22
Copy link
Author

qinzhang22 commented Nov 27, 2023

Please apply change request that I originally made in PR 18 to keep Python3 working: #18 (comment)

Here is Pau's hard disagree comment again: #15 (comment) Andrew's initiative to keep Python2 #15 (comment) and also Alex questioned splitting the code base of packages recently when there is no need to do so.

This change is small and even reduces the amount of changed lines:

try:
    import xmlrpclib
    import SocketServer
    import SimpleXMLRPCServer
except:
    import xmlrpc.client as xmlrpclib
    import socketserver as SocketServer
    import xmlrpc.server as SimpleXMLRPCServer

In addition, there are xsconsole plugins on xs8. You can find one with rpm -qa on xs8

rpm -qa | grep xsconsole

What about xsconsole plugins by partners? Can we check them?

Until these are checked and updated as well, we cannot declare xsconsole update work as finished.

We has a new agreement here: #18 (comment). That is move for Py3.6 baseline as there's no dependency of xsconsole that's going to need it to stay py2 compatible. And my PR is working on XS8. So what's your point we have to make such change here?

I don't know what partners use our xsconsole and how to check that. @rosslagerwall and @andyhhp Can you provide any information here?

'unicode' is not available in python3, so remove the use of it. And for
python3 'bytes' string might need to be converted to a regular string

Signed-off-by: Qin Zhang (张琴) <[email protected]>
Signed-off-by: Qin Zhang (张琴) <[email protected]>
We use PyPAM for python2 and as a result of testing the part of code is not working for
python3-pam with the failure "'PamAuthenticator' object has no attribute 'start'"

Signed-off-by: Qin Zhang (张琴) <[email protected]>
Signed-off-by: Qin Zhang (张琴) <[email protected]>
… a regular string in Python2

Signed-off-by: Qin Zhang (张琴) <[email protected]>
bernhardkaindl added a commit to xenserver-next/xsconsole that referenced this pull request Dec 2, 2023
bernhardkaindl added a commit to xenserver-next/xsconsole that referenced this pull request Dec 2, 2023
bernhardkaindl added a commit to xenserver-next/xsconsole that referenced this pull request Dec 3, 2023
bernhardkaindl added a commit to xenserver-next/xsconsole that referenced this pull request Dec 3, 2023
bernhardkaindl added a commit to xenserver-next/xsconsole that referenced this pull request Dec 4, 2023
bernhardkaindl added a commit to xenserver-next/xsconsole that referenced this pull request Dec 6, 2023
bernhardkaindl added a commit that referenced this pull request Dec 6, 2023
@bernhardkaindl
Copy link
Collaborator

Merged in split form with #24, #25, #28 and the change force the use of Python3 is no longer needed.

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.

6 participants