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.

Sign up for GitHub

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

Jump to bottom

Futurize the codebase for Python 3 support #424

Closed
wants to merge 1 commit into from
Closed

Futurize the codebase for Python 3 support #424

wants to merge 1 commit into from

Conversation

Conan-Kudo
Copy link

@Conan-Kudo Conan-Kudo commented Mar 18, 2019 •
edited
Loading

This PR attempts to make it Python 3 compatible. However, I don't know how to test this code, so I need some help validating whether it's working.

Fixes #423

@Conan-Kudo Conan-Kudo changed the title WIP: Futurize the codebase for Python 3 support Futurize the codebase for Python 3 support Mar 18, 2019
@Conan-Kudo
Copy link
Author

@breillyr have you had a chance to look at this?

@frantisekz
Copy link

frantisekz commented May 25, 2019 •
edited
Loading

@Conan-Kudo

You need to import from __future__ at the beginning of file, applying this after your PR patch makes imagefactory build:

diff --git a/imgfac/rest/bottle.py b/imgfac/rest/bottle.py
index 7795f05..5eaf9ee 100644
--- a/imgfac/rest/bottle.py
+++ b/imgfac/rest/bottle.py
@@ -13,6 +13,7 @@ Copyright (c) 2011, Marcel Hellkamp.
 License: MIT (see LICENSE.txt for details)
 """
 from __future__ import print_function
+from __future__ import with_statement
 
 from future import standard_library
 standard_library.install_aliases()
@@ -22,7 +23,6 @@ from builtins import zip
 from builtins import str
 from past.builtins import basestring
 from builtins import object
-from __future__ import with_statement
 
 __author__ = 'Marcel Hellkamp'
 __version__ = '0.10.9'
-- 
2.21.0

Anyway, whenever I try to build something, it ends with this traceback:

<h1>Critical error while processing request: /imagefactory/base_images</h1><h2>Error:</h2>
<pre>
TypeError(u&#039;Cannot mix str and non-str arguments&#039;,)
</pre>
<h2>Traceback:</h2>
<pre>
Traceback (most recent call last):
  File &quot;/usr/lib/python2.7/site-packages/imgfac/rest/bottle.py&quot;, line 833, in wsgi
    out = self._cast(self._handle(environ), request, response)
  File &quot;/usr/lib/python2.7/site-packages/imgfac/rest/bottle.py&quot;, line 786, in _cast
    out = self.error_handler.get(out.status, repr)(out)
  File &quot;/usr/lib/python2.7/site-packages/imgfac/rest/bottle.py&quot;, line 245, in __repr__
    return tonat(template(ERROR_PAGE_TEMPLATE, e=self))
  File &quot;/usr/lib/python2.7/site-packages/imgfac/rest/bottle.py&quot;, line 2803, in template
    return TEMPLATES[tpl].render(kwargs)
  File &quot;/usr/lib/python2.7/site-packages/imgfac/rest/bottle.py&quot;, line 2777, in render
    self.execute(stdout, kwargs)
  File &quot;/usr/lib/python2.7/site-packages/imgfac/rest/bottle.py&quot;, line 2765, in execute
    eval(self.co, env)
  File &quot;&lt;string&gt;&quot;, line 18, in &lt;module&gt;
  File &quot;/usr/lib/python2.7/site-packages/imgfac/rest/bottle.py&quot;, line 1053, in url
    return self.urlparts.geturl()
  File &quot;/usr/lib/python2.7/site-packages/future/backports/urllib/parse.py&quot;, line 252, in geturl
    return urlunsplit(self)
  File &quot;/usr/lib/python2.7/site-packages/future/backports/urllib/parse.py&quot;, line 399, in urlunsplit
    _coerce_args(*components))
TypeError: Cannot mix str and non-str arguments

</pre>

Please, ignore those html blocks in the traceback, this is how imagefactory prints it to stdout :/

I am testing your PR as if it was deployed on our Taskotron VMs (that means imagefactory and its TinMan plugin), you can see our scripts here: https://pagure.io/taskotron/base_images .

I can look more into this issue after the weekend if the traceback is not enough.

@hroncok
Copy link
Contributor

hroncok commented May 25, 2019

@frantisekz Just a note: future import of with_statement is only needed on Python 2.5. Anything older doesn't support it and anything newer already has it. Please, get rid of it entirely.

@hroncok
Copy link
Contributor

hroncok commented May 25, 2019

Note 2: Instead of porting the bundled bottle from February 2012, maybe just update it to a recent version or get rid of bundling it?

@frantisekz
Copy link

@hroncok Good catch, I didn't realize imagefactory was bundling bottle. It seems to work just fine after wiping the bundled version out and using bottle from python2-bottle (at least the Fedora build using TinMan, I don't know how to test the rest of imagefactory).

diff --git a/imagefactoryd b/imagefactoryd
index 6102c44..f643911 100755
--- a/imagefactoryd
+++ b/imagefactoryd
@@ -20,11 +20,11 @@ import os
 import os.path
 import signal
 import logging
+from bottle import *
 from imgfac.PersistentImageManager import PersistentImageManager
 from imgfac.ApplicationConfiguration import ApplicationConfiguration
 from imgfac.BuildDispatcher import BuildDispatcher
 from imgfac.Singleton import Singleton
-from imgfac.rest.bottle import *
 from imgfac.rest.RESTv2 import rest_api
 from imgfac.PluginManager import PluginManager
 from time import asctime, localtime
diff --git a/imgfac/rest/OAuthTools.py b/imgfac/rest/OAuthTools.py
index 5f2b2e3..95ed425 100644
--- a/imgfac/rest/OAuthTools.py
+++ b/imgfac/rest/OAuthTools.py
@@ -14,9 +14,9 @@
 #   limitations under the License.
 
 from builtins import object
+from bottle import *
 import logging
 import oauth2 as oauth
-from imgfac.rest.bottle import * 
 from imgfac.ApplicationConfiguration import ApplicationConfiguration
 
 log = logging.getLogger(__name__)
diff --git a/imgfac/rest/RESTtools.py b/imgfac/rest/RESTtools.py
index da3a344..286c140 100644
--- a/imgfac/rest/RESTtools.py
+++ b/imgfac/rest/RESTtools.py
@@ -14,8 +14,8 @@
 #   limitations under the License.
 
 import logging
+from bottle import *
 from imgfac.ApplicationConfiguration import ApplicationConfiguration
-from imgfac.rest.bottle import *
 from imgfac.picklingtools.xmlloader import *
 
 log = logging.getLogger(__name__)
diff --git a/imgfac/rest/RESTv2.py b/imgfac/rest/RESTv2.py
index 751dc4b..052502a 100644
--- a/imgfac/rest/RESTv2.py
+++ b/imgfac/rest/RESTv2.py
@@ -23,7 +23,7 @@ sys.path.insert(1, '%s/imgfac/rest' % sys.path[0])
 
 import logging
 import os.path
-from .bottle import *
+from bottle import *
 from imgfac.rest.RESTtools import *
 from imgfac.rest.OAuthTools import oauth_protect
 from imgfac.BuildDispatcher import BuildDispatcher

@Conan-Kudo Conan-Kudo closed this Jun 21, 2021
@Conan-Kudo
Copy link
Author

This is horribly out of date and imgfac supports python 3 (kind of) already...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Port to Python 3
3 participants
@Conan-Kudo @frantisekz @hroncok