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

cubetl/geoip uses incf.countryutils that doesn't support python 3 #6

Open
dfrankow opened this issue Nov 17, 2019 · 5 comments
Open
Assignees
Labels
bug feedback-needed Feedback from reporter or others is needed support

Comments

@dfrankow
Copy link
Contributor

dfrankow commented Nov 17, 2019

I am using python 3.7.4.

To reproduce:

$ docker-compose run cubetls python -c "import cubetl.geoip"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/app/cubetl/geoip/__init__.py", line 26, in <module>
    from incf.countryutils import transformations
  File "/usr/local/lib/python3.7/site-packages/incf/countryutils/transformations.py", line 151
    raise KeyError, code
                  ^
SyntaxError: invalid syntax

That package was last updated in 2009. There is a more recently updated version. It looks to not have the same error:

$ git clone https://github.com/wyldebeast-wunderliebe/incf.countryutils.git
$ cd incf.countryutils.git
$ virtualenv .venv
$ source .venv/bin/activate
$ python setup.py install
$ python -c "from incf.countryutils import transformations"
$ 
@dfrankow
Copy link
Contributor Author

FYI, if using the new package could fix this, you could change requirements.txt as follows:

diff --git a/requirements.txt b/requirements.txt
index 252a01e..fac124e 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -4,7 +4,8 @@ dateutils==0.6.6
 fake-factory==0.5.7
 fs==2.1.2
 httplib2==0.12.0
-incf.countryutils==1.0
+# updated incf.countryutils for Python 3
+https://github.com/wyldebeast-wunderliebe/incf.countryutils/zipball/1.1
 incf.dai==0.7
 lxml==4.2.5
 odict==1.6.2

However, I think geoip has other issues as well. I'm going to use this issue to write notes.

@dfrankow
Copy link
Contributor Author

dfrankow commented Nov 17, 2019

GeoIP requires libGeoIP. See maxmind/geoip-api-python#20.

Here are some instructions to install that (in the docker image I started):

apt update
apt -y install software-properties-common dirmngr apt-transport-https lsb-release ca-certificates
cd /root
add-apt-repository -y ppa:maxmind/ppa
apt install -y libgeoip1 libgeoip-dev geoip-bin
pip install GeoIP user_agents

See also https://github.com/maxmind/geoip-api-c#on-ubuntu-using-ppa.

However, note geoip is deprecated, and they now recommend geoip2. :/

@dfrankow
Copy link
Contributor Author

Note even after installing libGeoIP and GeoIP, the test_loganalyzer test fails with some other issue:

self = <sqlalchemy.engine.strategies.PlainEngineStrategy object at 0x7f05956a72d0>, name_or_url = None
kwargs = {'connect_args': {}}, u = None

    def create(self, name_or_url, **kwargs):
        # create url.URL object
        u = url.make_url(name_or_url)
    
>       plugins = u._instantiate_plugins(kwargs)
E       AttributeError: 'NoneType' object has no attribute '_instantiate_plugins'

/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/strategies.py:52: AttributeError
================================================ warnings summary ================================================
cubetl/olap/sqlschema.py:415
  /app/cubetl/olap/sqlschema.py:415: DeprecationWarning: invalid escape sequence \.
    pattern = ci_key.replace('.', '\.').replace('**', '.*').replace('*', '\w+')

cubetl/olap/sqlschema.py:415
  /app/cubetl/olap/sqlschema.py:415: DeprecationWarning: invalid escape sequence \w
    pattern = ci_key.replace('.', '\.').replace('**', '.*').replace('*', '\w+')

tests/test_examples.py::TestExamples::test_loganalyzer
  /usr/local/lib/python3.7/site-packages/beautifulsoup4-4.8.1-py3.7.egg/bs4/dammit.py:53: DeprecationWarning: invalid escape sequence \s
    xml_encoding = '^\s*<\\?.*encoding=[\'"](.*?)[\'"].*\\?>'

-- Docs: https://docs.pytest.org/en/latest/warnings.html

I don't know what's up here.

@jjmontesl jjmontesl self-assigned this Nov 20, 2019
@jjmontesl jjmontesl added the bug label Nov 20, 2019
@jjmontesl
Copy link
Owner

The documentation of cubetl.geoip.GeoIPFromAddress node says that:

Note: This module uses incf.countryutils, but as the version in PyPI does not currently
support Python 3, the updated package for Python 3 is included by this project (see
https://github.com/wyldebeast-wunderliebe/incf.countryutils/issues/2 ).

Since I had already copied a working countryutils copy from that repository, try removing the offending package from your environment:

pip uninstall incf.countryutils

I have removed this dependency from the requirements.txt file.

I wouldn't like to make the project depend on geoip. These dependencies shall be installed by users if they need them, that's why you have to install GeoIP manually (as per the docs). This should perhaps be discovered by the cubetl.geoip module and provide a meaningful error.

@jjmontesl
Copy link
Owner

I have pushed the corrections to the master branch, the loganalyzer example now works for me given the above works.

@jjmontesl jjmontesl added support feedback-needed Feedback from reporter or others is needed labels Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feedback-needed Feedback from reporter or others is needed support
Projects
None yet
Development

No branches or pull requests

2 participants