#2 Token authentication
Merged by abompard. Opened by atelic.
atelic/plus-plus-service add-tokens  into  master

Download 2.patch
  • Token authentication and accompanying tests
  • Flake8 fixes

2 new commits added

  • Fix flake8 errors, py 2 compatability, add tests
  • Token authentication

2 new commits added

  • Fix flake8 errors, py 2 compatability, add tests
  • Token authentication

Why not use Flask's configuration system? (APP.config)

Is this still necessary in Python 3? I don't think so.

Same remark, this should not be necessary in Python3

The super() form is preferred in Python3.

Same remark, the super() form is preferred in Python3.

Same comment here, using the Flask config system seems better (APP.config)

Same comment, the super() form is preferred in Python3.

Same comment, the super() form is preferred in Python3.

This is intended to be a Python3 app.

This is intended to be a Python3 app.

Versionned requirements make apps much much harder to package, I would recommend against it.

About Python2 vs Python3, I think there's a discussion to have here. I think it's much trickier to try to retain Python2 compatibility, we would also have to add imports from future at the top of every file (from __future__ import unicode_literals, absolute_import, with_statement, print_statement at least and maybe others).

From the Mailman experience I think it's much safer to target Python3 if we know we have Python3 on the target OS, and that is our case. With our use case, I think it's not very useful to try to support Python2 today, it will only adds support issues.

I spoke with Pierre about the Python2 vs Python3 concerns on #fedora-apps. He said to aim for py2 compatibility since that is the version that is default on RHEL and F24. You're welcome to bring these concerns up to him. However, the application does seem to run on python 2 and 3 locally without the future imports.

That said, I agree that it feels like building for the Windows XP of programming languages. I do think it would be preferable to build new applications in py3 rather than continuing to build things in a version that will be losing support soon.

Yeah, it probably runs without the future imports, but it can bring very convoluted bugs. The nastiest is unicode support: without the future import all the strings in the code are binary strings, but in Python3 (or with the future import) they are unicode strings, and must be converted to binary appropriately. For now Flask does the work for us, but since we'll also be sending fedmsgs, we may have to be extra careful about the status of our strings.

Same thing for absolute imports, you can have a running code in Python3 that will stop working in Python2, or vice-versa, because the name of a submodule is identical to one shipped in base Python (2 or 3). My advice from the Mailman experience is to choose a version and stick to it, unless you have to (for example if you're writing a library that must be compatible).

@pingou , what's your opinon?

Can you explain why? I'm not sure I understand this.

Builds should be stable over time. If a project builds now, it shouldn’t break in the future. Unversioned requirements do not ensure that one build will use the same version that the app was developed with and if a new version is released with breaking changes this will break the application.

Yeah, I know this is a cause of debate, usually between developers and packagers/sysadmins. Here are the use cases:

  • if there's a new version of requests for example, that fixes a security issue, the packager of requests will update the package, and it will break our application (to be precise, the updated package won't be installable unless plus-plus-service is removed)
  • if we want to run pps on a distro version that have different but working version of the libraries, the package won't install and the app won't run
  • it's a pain to maintain, but that's less important than the two previous points

I don't have a perfect solution for this problem, it is a very common issue in software engineering. I usually set minimal versions where I know it needs a recent version.

Our Fedora Packaging Guidelines recommend not setting a version when it is not necessary: http://fedoraproject.org/wiki/Packaging:Guidelines#Package_dependencies

3 new commits added

  • Use APP.config for token configuration
  • Fix flake8 errors, add tests
  • Token authentication

3 new commits added

  • Use APP.config for token configuration
  • Fix flake8 errors, add tests
  • Token authentication

Pull-Request has been merged by abompard

Metadata