[GitHub] john-bodley commented on issue #4552: [travis/tox] Restructuring configuration and testing

2018-04-08 Thread GitBox
john-bodley commented on issue #4552: [travis/tox] Restructuring configuration 
and testing
URL: 
https://github.com/apache/incubator-superset/pull/4552#issuecomment-379573029
 
 
   @mistercrunch per our conversation last week, I updated `CONTRIBUTING.md` 
regarding maintaining `setup.py` and `requirements.txt`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] john-bodley commented on issue #4552: [travis/tox] Restructuring configuration and testing

2018-04-08 Thread GitBox
john-bodley commented on issue #4552: [travis/tox] Restructuring configuration 
and testing
URL: 
https://github.com/apache/incubator-superset/pull/4552#issuecomment-379573029
 
 
   @mistercrunch per our conversation last week, I updated CONTRIBUTING.md 
regarding maintaining setup.py and requirements.txt.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] john-bodley commented on issue #4552: [travis/tox] Restructuring configuration and testing

2018-04-05 Thread GitBox
john-bodley commented on issue #4552: [travis/tox] Restructuring configuration 
and testing
URL: 
https://github.com/apache/incubator-superset/pull/4552#issuecomment-378998654
 
 
   @mistercrunch it would be good to understand how Lyft creates these 
different files (though your hypothesis seems valid). It seems you maybe able 
to achieve this in conjunction with pyenv. I can also explore pipenv, though it 
would need to play well with tox, Travis, etc. 
   
   Also if we went down the virtualenv route one would need to activate it for 
the various services or wrap existing command via pipenv run.
   
   Finally the setup.py/requirements.txt relationship is well defined, in 
essence setup.py should never pin packages but merely provide the list of 
required dependencies, with version restrictions were appropriate. I realize 
that Superset is an application rather than a package, but this allows 
individual instances to configure their Python environment how they see 
appropriate, i.e., a custom plugin may require a different version of 
dependency. The requirements.txt simply provides a deterministic environment 
using package versions which we’ve certified/tested.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] john-bodley commented on issue #4552: [travis/tox] Restructuring configuration and testing

2018-04-05 Thread GitBox
john-bodley commented on issue #4552: [travis/tox] Restructuring configuration 
and testing
URL: 
https://github.com/apache/incubator-superset/pull/4552#issuecomment-378998654
 
 
   @mistercrunch it would be good to understand how Lyft creates these 
different files and I can explore pipenv, though it would need to play well 
with tox, Travis, etc. 
   
   Also if we went down the virtualenv route one would need to activate it for 
the various services or wrap existing command via pipenv run.
   
   Finally the setup.py/requirements.txt relationship is well defined, in 
essence setup.py should never pin packages but merely provide the list of 
required dependencies, with version restrictions were appropriate. I realize 
that Superset is an application rather than a package, but this allows 
individual instances to configure their Python environment how they see 
appropriate, i.e., a custom plugin may require a different version of 
dependency. The requirements.txt simply provides a deterministic environment 
using package versions which we’ve certified/tested.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] john-bodley commented on issue #4552: [travis/tox] Restructuring configuration and testing

2018-04-05 Thread GitBox
john-bodley commented on issue #4552: [travis/tox] Restructuring configuration 
and testing
URL: 
https://github.com/apache/incubator-superset/pull/4552#issuecomment-378831814
 
 
   @mistercrunch I definitely explored the `pip-compile` route as it would be 
great to have `requirements.txt` autogenerated from `setup.py` and 
`requirements-dev.txt` autogenerated from `requirements-dev.in` etc. 
   
   The issue is there's not a plausible way to autogenerate a Python 2/3 
compatible `requirements.txt` file, i.e., in Python 2.7 `pip-compile` generates:
   ```
   #
   # This file is autogenerated by pip-compile
   # To update, run:
   #
   #pip-compile --output-file requirements.txt setup.py
   #
   alembic==0.9.9# via flask-migrate
   amqp==2.2.2   # via kombu
   asn1crypto==0.24.0# via cryptography
   babel==2.5.3  # via flask-babel, flower
   backports-abc==0.5# via tornado
   billiard==3.5.0.3 # via celery
   bleach==2.1.3
   boto3==1.7.0
   botocore==1.10.0  # via boto3, s3transfer
   celery==4.1.0
   certifi==2018.1.18# via requests
   cffi==1.11.5  # via cryptography
   chardet==3.0.4# via requests
   click==6.7# via flask, flask-appbuilder
   colorama==0.3.9
   cryptography==2.2.2
   docutils==0.14# via botocore
   enum34==1.1.6 # via cryptography
   flask-appbuilder==1.10.0
   flask-babel==0.11.1   # via flask-appbuilder
   flask-cache==0.13.1
   flask-compress==1.4.0
   flask-login==0.2.11   # via flask-appbuilder
   flask-migrate==2.1.1
   flask-openid==1.2.5   # via flask-appbuilder
   flask-script==2.0.6
   flask-sqlalchemy==2.1
   flask-testing==0.7.1
   flask-wtf==0.14.2
   flask==0.12.2
   flower==0.9.2
   future==0.16.0
   futures==3.2.0# via flower, s3transfer, tornado
   geopy==1.12.0
   gunicorn==19.7.1
   html5lib==1.0.1   # via bleach
   humanize==0.5.1
   idna==2.6
   ipaddress==1.0.19 # via cryptography
   itsdangerous==0.24# via flask
   jinja2==2.10  # via flask, flask-babel
   jmespath==0.9.3   # via boto3, botocore
   kombu==4.1.0  # via celery
   mako==1.0.7   # via alembic
   markdown==2.6.11
   markupsafe==1.0   # via jinja2, mako
   numpy==1.14.2 # via pandas
   pandas==0.22.0
   parsedatetime==2.4
   pathlib2==2.3.0
   polyline==1.3.2
   pycparser==2.18   # via cffi
   pydruid==0.4.2
   pyhive==0.5.1
   python-dateutil==2.6.1
   python-editor==1.0.3  # via alembic
   python-geohash==0.8.5
   python-openid==2.2.5  # via flask-openid
   pytz==2018.3  # via babel, celery, flower, pandas
   pyyaml==3.12
   requests==2.18.4
   s3transfer==0.1.13# via boto3
   sasl==0.2.1   # via thrift-sasl
   scandir==1.7  # via pathlib2
   simplejson==3.13.2
   singledispatch==3.4.0.3   # via tornado
   six==1.11.0
   sqlalchemy-utils==0.33.2
   sqlalchemy==1.2.6
   sqlparse==0.2.4
   thrift-sasl==0.3.0
   thrift==0.11.0
   tornado==5.0.1# via flower
   unicodecsv==0.14.1
   unidecode==1.0.22
   urllib3==1.22 # via requests
   vine==1.1.4   # via amqp
   webencodings==0.5.1   # via html5lib
   werkzeug==0.14.1  # via flask
   wtforms==2.1  # via flask-wtf
   ```
   however if you try to install this in Python 3 via, `pip3 install -r 
requirements.txt` it fails with the following error:
   ```
   Collecting futures==3.2.0 (from -r requirements.txt (line 33))
 Could not find a version that satisfies the requirement futures==3.2.0 
(from -r requirements.txt (line 33)) (from versions: 0.2.python3, 0.1, 0.2, 
1.0, 2.0, 2.1, 2.1.1, 2.1.2, 2.1.3, 2.1.4, 2.1.5, 2.1.6, 2.2.0, 3.0.0, 3.0.1, 
3.0.2, 3.0.3, 3.0.4, 3.0.5, 3.1.0, 3.1.1)
   No matching distribution found for futures==3.2.0 (from -r requirements.txt 
(line 33))
   ```
   I thought of two possible solutions:
   1. Enabling `pip-compile` to support multiple Python versions, which is 
something that `requirements.txt supports, i.e.,
   ```
   futures==3.2.0; python_version == '2.7'
   ```
   2. Enabling `pip-compile` to exclude dependencies (which `futures` is) which 
would make the output Python 2/3 compatible.
   
   I raised this [issue](https://github.com/jazzband/pip-tools/issues/639) with 
`pip-tools` where the suggestion was to create a `requirements.txt` for each 
environment, however I'm not certain we want to have to build/maintain 
environment specific files.



This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] john-bodley commented on issue #4552: [travis/tox] Restructuring configuration and testing

2018-04-04 Thread GitBox
john-bodley commented on issue #4552: [travis/tox] Restructuring configuration 
and testing
URL: 
https://github.com/apache/incubator-superset/pull/4552#issuecomment-378790730
 
 
   @mistercrunch sorry I was wondering what your thoughts were on this? 
Additionally having a `requirements.txt` with pinned versions would help us 
with caching Python packages in Docker.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] john-bodley commented on issue #4552: [travis/tox] Restructuring configuration and testing

2018-03-23 Thread GitBox
john-bodley commented on issue #4552: [travis/tox] Restructuring configuration 
and testing
URL: 
https://github.com/apache/incubator-superset/pull/4552#issuecomment-375824124
 
 
   @mistercrunch @betodealmeida per the conservation regarding pinning earlier 
today what are your thoughts on this PR? Note beyond `requirements.txt` there's 
changes in configuration around testing (including unit tests), the `tox` test 
harness, and Travis CI. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services