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

Remove depreacted use2to3 option from setup.py #60

Merged
merged 1 commit into from
Sep 8, 2021

Conversation

opus-42
Copy link

@opus-42 opus-42 commented Sep 7, 2021

Since Septembre 6th, 2021, the use of the setup option is no longer accepted and leads to failed builds. This is reported in issue #59.

This pull request proposes to removing this option from setup.py.

I didn't see any test suite though, so I couldn't should make sure all the code is compatible with newest version of Python (which seems so).

@nikhil-2011
Copy link

I am also getting the same error while building the pipelines - error in Flask-OpenID setup command: use_2to3 is invalid.
Could you please let me know ETA for this issue to be resolved as this is impacting our production environment.

@getumen
Copy link

getumen commented Sep 7, 2021

@nikhil-2011 you can buy time by using older version of setuptools like pip install "setuptools<58.0.0".
Of course, solving this issue is better.

@nikhil-2011
Copy link

Thanks for the suggestion @getumen , Is there any other way out to solve this issue or this will be corrected by upstream team?

@twcurrie
Copy link

twcurrie commented Sep 7, 2021

This deprecation is an important one to fix.

@tatiana
Copy link

tatiana commented Sep 8, 2021

There was a change to setuptools and a new version was released a few hours ago (58.0.3), but it only works if extra['use_2to3'] = False:
pypa/setuptools#2777

It would be great if we could have a new release of flask-openid including the changes in this PR by @opus-42! I wonder if either @mitsuhiko or @puiterwijk might be able to help us on this one.

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

Yep. That would really be helpful - as it breaks pretty much all historical installations of Airflow, and there is basically nothing we can do about it unless a new version of flask-opeind is released without the option. @mitsuhiko @puiterwijk - any chance for that ? or maybe you give access to someone (happy to do so) in GitHub/PyPI to rebuild/release it.

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

FYI. We have forked and created a release in apache/airlfow (we were able to workaround the problem because we are using constraints mechanism to install airflow):

So the .tar.gz is there - https://github.com/apache/airflow-flask-openid-fork/releases/tag/v1.2.6

We literally need to release it to PyPI to solve it for others too @mitsuhiko @puiterwijk ?

The Airflow Issue where we track it is here: apache/airflow#18075

dpgaspar added a commit to dpgaspar/Flask-AppBuilder that referenced this pull request Sep 8, 2021
@mitsuhiko
Copy link
Collaborator

Does this codebase even work without 2to3? I haven't touched it in years.

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

First of all thanks for coming back @mitsuhiko ! I do realise it's pretty much "abandoned" for you and you are more into Rust than Python now, but I appreciate, you get back even after so long time.

The problem was not about the code and it's functionality - we could likely very easilly incorporate it - either to FlaskAppBuilder or Airflow (I am talking now about the two impacted projects that I know of). But more of the problem is that it breaks already released versions of our software (of course it's because of setuptools, not flask-openid, but well - it breaks).

It looks like it should work. I believe 2to3 was not introducing any serious changes, just removing some extra stuff that is not needed any more in Python 3 - not at least in the last version. And yeah. I will defer to @dpgaspar to test but py_compile works, it does not show any incompatibilites, it imports well, and the code does not seem to contain any python2-only constructs. I also run 2to3 on the module (there is really just one short module) and I see no serious problems (see below).

I could actually very easily release a package "after" pre-processing it by 2to3 and I could change requirements to Python 3 only to be absolutely sure (happy to take on the task if you can trust me :) ).

--- flask_openid.py     (original)
+++ flask_openid.py     (refactored)
@@ -10,7 +10,7 @@
     :copyright: (c) 2010 by Armin Ronacher.
     :license: BSD, see LICENSE for more details.
 """
-from __future__ import absolute_import
+
 
 import os
 import pickle
@@ -109,7 +109,7 @@
     if sys.version_info[0] >= 3:
         return isinstance(x, str)
     else:
-        return isinstance(x, basestring)
+        return isinstance(x, str)
 
 
 class SessionWrapper(object):
@@ -210,8 +210,8 @@
         self.fullname = lookup.get_combined('fullname', FULL_NAME_URIS)
         if self.fullname is None:
             first = lookup.get_uri('http://axschema.org/namePerson/first')
-            last = lookup.get_uri(u'http://axschema.org/namePerson/last')
-            self.fullname = u' '.join(x for x in [first, last] if x) or None
+            last = lookup.get_uri('http://axschema.org/namePerson/last')
+            self.fullname = ' '.join(x for x in [first, last] if x) or None
 
         #: desired nickname of the user
         self.nickname = lookup.get('nickname')
@@ -496,7 +496,7 @@
         """
         @wraps(f)
         def decorated(*args, **kwargs):
-            if request.args.get('openid_complete') != u'yes':
+            if request.args.get('openid_complete') != 'yes':
                 return f(*args, **kwargs)
             consumer = Consumer(SessionWrapper(self), self.store_factory())
             args = request.args.to_dict()
@@ -506,16 +506,16 @@
                 return self.after_login_func(OpenIDResponse(
                     openid_response, self.extension_responses))
             elif openid_response.status == CANCEL:
-                self.signal_error(u'The request was cancelled')
+                self.signal_error('The request was cancelled')
             elif openid_response.status == FAILURE:
-                self.signal_error(u'OpenID authentication failure. Mesage: %s'
+                self.signal_error('OpenID authentication failure. Mesage: %s'
                                   % openid_response.message)
             elif openid_response.status == SETUP_NEEDED:
                 # Unless immediate=True, we should never get here
-                self.signal_error(u'OpenID setup was needed')
+                self.signal_error('OpenID setup was needed')
             else:
                 # We should also never get here, as this should be exhaustive
-                self.signal_error(u'OpenID authentication weird state: %s' %
+                self.signal_error('OpenID authentication weird state: %s' %
                                   openid_response.status)
             return redirect(self.get_current_url())
         return decorated
@@ -565,7 +565,7 @@
                 for extension in extensions:
                     auth_request.addExtension(extension)
         except discover.DiscoveryFailure:
-            self.signal_error(u'The OpenID was invalid')
+            self.signal_error('The OpenID was invalid')
             return redirect(self.get_current_url())
         if self.url_root_as_trust_root:
             trust_root = request.url_root

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

Just getting access to GitHub and PyPI to release it would be enough. Releasing 1.2.6 with Python 3 only compatibility afer processing it manually by 2to3 is something I am happy to do (just in case both IDs for GitHub and PyPI for me is @potiuk and you can likely check - especially my Airflow activity if you are afraid if you can trust me :).

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

I own ~ 140 packages on pypi (all apache-airflow related :D)

@jaraco
Copy link
Collaborator

jaraco commented Sep 8, 2021

It looks like it should work. I believe 2to3 was not introducing any serious changes, just removing some extra stuff that is not needed any more in Python 3 - not at least in the last version. And yeah. I will defer to @dpgaspar to test but py_compile works, it does not show any incompatibilites, it imports well, and the code does not seem to contain any python2-only constructs. I also run 2to3 on the module (there is really just one short module) and I see no serious problems (see below).

Looks like basestring is referenced, which isn't a builtin on Python 3, so it will require a code change to support native Python 3 compatibility.

@mitsuhiko I'm also willing to share ownership of the project. Let us know what you'd like to see for it. I'm interested in getting this fixed as it's the most prominent blocker to avoid reverting Setuptools 58.

@jaraco
Copy link
Collaborator

jaraco commented Sep 8, 2021

#61 extends this PR to apply the Python 3 only changes.

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

Looks like basestring is referenced, which isn't a builtin on Python 3, so it will require a code change to support native Python 3 compatibility.

basestring is only used in if Python <3 branch

@mitsuhiko
Copy link
Collaborator

Happy to hand this over to someone who wants to take care of it.

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

I volunteer :) @mitsuhiko

@mitsuhiko
Copy link
Collaborator

mitsuhiko commented Sep 8, 2021

I gave you both access to this repository. Please send me an email / leave a comment for pypi access.

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

sent

@mitsuhiko
Copy link
Collaborator

Added

@potiuk potiuk merged commit 43224f9 into pallets-eco:master Sep 8, 2021
@uranusjr
Copy link

uranusjr commented Sep 8, 2021

Would it be possible to grant me PyPI access? I’m also an Airflow maintainer and can help publish some wheels for the project. (Actually already have the 1.3.0 one ready.)

My PyPI account name is the same as GitHub (@uranusjr).

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

Ah. too late @uranusjr - I just released them :)

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

Brand new https://pypi.org/project/Flask-OpenID/1.3.0/ released now!

@uranusjr
Copy link

uranusjr commented Sep 8, 2021

Still without wheels though 😉

@kaxil
Copy link

kaxil commented Sep 8, 2021

Would be good to get wheels too

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

All right, all right @kaxil @uranusjr . Wheel is there now :)

@potiuk
Copy link
Collaborator

potiuk commented Sep 8, 2021

Thanks again @mitsuhiko ! That is really helpful and thanks for being so responsive and open! It's not often, even in open-source world !

@zachliu
Copy link

zachliu commented Sep 20, 2021

I just encountered this issue #59 Monday morning and a Google search led me here.
Now I'm like All hail the opensource community!!! 🙏 🙏 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.