#178 Fix string type check for Python 2
Closed 2 years ago by lholecek. Opened 2 years ago by lholecek.
lholecek/waiverdb fix-str-type-check  into  master

file modified
@@ -26,3 +26,6 @@ 


  # Database



+ # Python 2 compatibility

+ six

file modified
@@ -43,6 +43,7 @@ 

  BuildRequires:  python3-flask-migrate

  BuildRequires:  python3-stomppy

  BuildRequires:  python3-fedmsg

+ BuildRequires:  python3-six

  BuildRequires:  python-flask

  BuildRequires:  python-sqlalchemy

  BuildRequires:  python-flask-restful

file modified
+3 -1
@@ -2,6 +2,8 @@ 


  import json


+ from six import string_types


  import requests

  from flask import Blueprint, request, current_app

  from flask_restful import Resource, Api, reqparse, marshal_with, marshal
@@ -37,7 +39,7 @@ 

  def _validate_results_filter(results):

      expected = {

          'subject': dict,

-         'testcase': str,

+         'testcase': string_types,


      for item in results:

          for k, v in item.items():

no initial comment


edit: no actually I'm having this error trying to call the: api/v1.0/waivers/+by-subjects-and-testcases

I get:
"message": {
"results": "name 'unicode' is not defined"

rebased onto 17a6e3f3c3f16a94585d99504e8144f965701105

2 years ago

I fixed the unicode problem. Though it adds six python module dependency.

All this is not even needed if Python 2 doen't have to be supported.

Eh, since six is a common dep I figure it is fine to keep.

There's not much standing in the way of us going py3-only though. I think the only outstanding piece is waiverdb-cli support on el7 for RHEL packagers. In another PR, we can/should split out the cli to free the rest of waiverdb from py2 concerns.

Yeah we only recently took out the dep on six :-) because we are living in the future! And we can be Py3-only. So I think we don't need this.

The CLI is indeed a wrinkle but as Ralph notes I would like to just fully separate that out anyway.

OK, I'm closing this since Python 2 support is not needed here.

Pull-Request has been closed by lholecek

2 years ago