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

Setuptools packaging improvments #1647

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions fixed-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
eventlet>=0.13.0
9 changes: 5 additions & 4 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# 1. Don't include forget to populate spcific requirements.txt. IMORTANT!!!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: typo. specific. IMPORTANT.
Nit: Sentence isn't making sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha-ha. You are right) This sentence will likely disappear from here soon,
let's ignore it meanwhile)

On Tue, Jun 23, 2015, 07:53 Lakshmi Kannan [email protected] wrote:

In requirements.txt
#1647 (comment):

@@ -1,3 +1,5 @@
+# 1. Don't include forget to populate spcific requirements.txt. IMORTANT!!!

Nit: typo. specific. IMPORTANT.
Nit: Sentence isn't making sense.


Reply to this email directly or view it on GitHub
https://github.com/StackStorm/st2/pull/1647/files#r33009336.

# 2. Don't include setuptools into any of requirements.txt (we don't want it bundled).
apscheduler>=3.0.0rc1
eventlet>=0.13.0
flask
Expand All @@ -15,19 +17,18 @@ python-dateutil
python-json-logger
pyyaml
requests
setuptools==11.1
six==1.9.0
tooz
git+https://github.com/StackStorm/[email protected]
git+https://github.com/StackStorm/fabric.git@stanley-patched
git+https://github.com/StackStorm/[email protected]#egg=python-mistralclient
git+https://github.com/StackStorm/fabric.git@stanley-patched#egg=fabric
passlib>=1.6.2,<1.7
lockfile>=0.10.2,<0.11
python-gnupg>=0.3.7,<0.4
jsonpath-rw>=1.3.0
# Requirements for linux pack
# used by file watcher sensor
pyinotify>=0.9.5,<=0.10
-e git+https://github.com/Kami/logshipper.git@stackstorm_patched#egg=logshipper
git+https://github.com/Kami/logshipper.git@stackstorm_patched#egg=logshipper
# used by nmap actions
python-nmap>=0.3.4,<0.4
semver>=2.1.2
134 changes: 134 additions & 0 deletions scripts/fixate-requirements.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
#!/usr/bin/env python
# Licensed to the StackStorm, Inc ('StackStorm') under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""
This script is used to automate generation of requirements.txt
for st2 components.

The idea behind this script is that that each component has it's own requirements
in-requirements.txt file (input requirements file). Except this file
there's also the top-level fixed-requirements.txt which pins production versions
for the whole st2 stack. During production use (building, packaging, etc)
requirements.txt is generated from in-requirements.txt where version of packages are
fixed according to fixed-requirements.txt.
"""

import argparse
import os
import os.path
import sys
from distutils.version import StrictVersion

OSCWD = os.path.abspath(os.curdir)
GET_PIP = ' curl https://bootstrap.pypa.io/get-pip.py | python'

try:
import pip
from pip.req import parse_requirements
except ImportError:
print 'Download pip:\n', GET_PIP
sys.exit(1)


def parse_args():
parser = argparse.ArgumentParser(description='Tool for requirements.txt generation.')
parser.add_argument('-s', '--source-requirements', nargs='+',
required=True,
help='Specifiy paths to requirements file(s). '
'In case severasl requirements files are given their content is merged.')
parser.add_argument('-f', '--fixed-requirements', required=True,
help='Specifiy path to fixed-requirements.txt file.')
parser.add_argument('-o', '--output-file', default='requirements.txt',
help='Specifiy path to the resulting requirements file.')
if len(sys.argv) < 2:
parser.print_help()
sys.exit(1)
return vars(parser.parse_args())


def check_pip_version():
if StrictVersion(pip.__version__) < StrictVersion('6.0.0'):
print "Upgrade pip, your version `{}' "\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Can you use print as methods? Also, we use '' for strings. PEP-8.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure. Got it. Ruby had known distinctions between ' and ", that's why
I automatically chosen quotes without thinking.

On Tue, Jun 23, 2015, 07:58 Lakshmi Kannan [email protected] wrote:

In scripts/fixate-requirements.py
#1647 (comment):

+cwd = os.path.abspath(os.getcwd())
+FIXED_REQF = os.path.join(cwd, '../fixed-requirements.txt')
+LOCAL_REQF = os.path.join(cwd, 'requirements.txt')
+GET_PIP = " curl https://bootstrap.pypa.io/get-pip.py

| python"

+try:

  • import pip
  • from pip.req import parse_requirements
    +except ImportError:
  • print "Download pip:\n", GET_PIP
  • sys.exit(1)

+def check_pip_version():

  • if StrictVersion(pip.version) < StrictVersion('7.0.0'):
  •    print "Upgrade pip, your version `{}' "\
    

Nit: Can you use print as methods? Also, we use '' for strings. PEP-8.


Reply to this email directly or view it on GitHub
https://github.com/StackStorm/st2/pull/1647/files#r33009484.

"is outdated:\n".format(pip.__version__), GET_PIP
sys.exit(1)


def load_requirements(file_path):
return tuple((r for r in parse_requirements(file_path, session=False)))


def locate_file(path, must_exist=False):
if not os.path.isabs(path):
path = os.path.join(OSCWD, path)
if must_exist and not os.path.isfile(path):
print("Error: couldn't locate file `{}'".format(path))
return path


def merge_source_requirements(sources):
"""Read requirements source files and merge it's content.
"""
projects = set()
merged_requirements = []
for infile_path in (locate_file(p, must_exist=True) for p in sources):
for req in load_requirements(infile_path):
# Requirements lines like "project[version_spec]"
if req.req:
# Skip already added project name
if req.req.project_name in projects:
continue
projects.add(req.req.project_name)
merged_requirements.append(req)

# Requirements lines like "vcs+proto://url"
elif req.link:
merged_requirements.append(req)
else:
raise RuntimeError('Unexpected requirement {}'.format(req))

return merged_requirements


def write_requirements(sources=None, fixed_requirements=None, output_file=None):
"""Wrire resulting requirements taking versions from the fixed_requirements.
"""
requirements = merge_source_requirements(sources)
fixed = load_requirements(locate_file(fixed_requirements, must_exist=True))
fixedreq_hash = {req.req.project_name: req for req in fixed if req.req}

with open(output_file, 'w') as f:
f.write("# Don't edit this file. It's generated automatically!\n")
for req in requirements:
# we don't have any idea how to process links, so just add them
if req.link:
rline = str(req.link)
else:
project = req.req.project_name
if project in fixedreq_hash:
rline = str(fixedreq_hash[project])
else:
rline = str(req.req)
f.write(rline + '\n')

return


if __name__ == '__main__':
check_pip_version()
args = parse_args()
write_requirements(sources=args['source_requirements'],
fixed_requirements=args['fixed_requirements'],
output_file=args['output_file'])
1 change: 1 addition & 0 deletions st2_version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0.10dev0
7 changes: 7 additions & 0 deletions st2api/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
eventlet>=0.13.0
jsonschema>=2.3.0
kombu
mongoengine>=0.8.7,<0.9
oslo.config
pecan==0.7.0
six==1.9.0
6 changes: 0 additions & 6 deletions st2api/setup.cfg

This file was deleted.

50 changes: 33 additions & 17 deletions st2api/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,43 @@
# See the License for the specific language governing permissions and
# limitations under the License.

try:
from setuptools import setup, find_packages
except ImportError:
from ez_setup import use_setuptools
use_setuptools()
from setuptools import setup, find_packages
import re
import os.path
from pip.req import parse_requirements
from setuptools import setup, find_packages


def fetch_requirements():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pip.req.parse_requirements should work as long as a new version of pip is available on the system.

See tweepy/tweepy#533 for an example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: Nvm, I didn't see the full diff. You already use that function :)

On a related note - we just need to make sure we use pip >= 6.0 since older versions don't take session argument.

links = []
reqs = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this short form for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some requirements are represented as git+proto//URL@branch#some_egg_dep or
other (of course I hope not to see svn in there :) these will be stored in
links array.

The links array is given to setup as a separate argument (if I'm not wrong
dependency_list. When setuptools want to locate some_egg_dep it will
look through the links array first and only after fallback to PyPI.

The reqs array contains someeggg, =ver requirements and similar.

On Tue, Jun 23, 2015, 08:01 Lakshmi Kannan [email protected] wrote:

In st2api/setup.py
#1647 (comment):

@@ -14,27 +14,43 @@

See the License for the specific language governing permissions and

limitations under the License.

-try:

  • from setuptools import setup, find_packages
    -except ImportError:
  • from ez_setup import use_setuptools
  • use_setuptools()
  • from setuptools import setup, find_packages
    +import re
    +import os.path
    +from pip.req import parse_requirements
    +from setuptools import setup, find_packages

+def fetch_requirements():

  • links = []
  • reqs = []

What is this short form for?


Reply to this email directly or view it on GitHub
https://github.com/StackStorm/st2/pull/1647/files#r33009564.

for req in parse_requirements('requirements.txt', session=False):
if req.link:
links.append(str(req.link))
reqs.append(str(req.req))
return (reqs, links)


current_dir = os.path.dirname(os.path.realpath(__file__))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, why was this removed?

st2api does include some non-source code data (HTML templates).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed that since, install_requires is not needed as we can use
load of requirements using pip. And if you mean package_data, since
we've got no exact and working way to select need files I started to
use MANIFEST.in where simply all files from the source directory are
included.

On Mon, Jun 22, 2015 at 2:46 PM, Tomaz Muraus
[email protected] wrote:

In st2api/setup.py:

 author='StackStorm',
 author_email='[email protected]',
  • install_requires=[
  •    "pecan",
    
  • ],
  • package_data={
    Hm, why was this removed?

st2api does include some non-source code data (HTML templates).


Reply to this email directly or view it on GitHub.

version_file = os.path.join(current_dir, '../st2client/st2client/__init__.py')
with open(version_file, 'r') as f:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this a bit more robust - can we just temporary add st2client package to the path and then do something like:

from st2client import __version__
version = __version__.split('.')

This should work as long as init.py in st2client doesn't depend on other external dependencies (which is a bad practice anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry but we can't use it as well as my method with parsing file. This limit is due to our potential tooling when wheel is created (or egg) - pip is run and component directory copied under some '/tmp/$RANDOM$' path.
That's why we can't use relative file read as well as your approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added top st2_version file, which is being populated for each component also if it can not be found. This is done by make, so by the type setup.py is run, all is nice file can be cat.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, dict comprehensions are only available in 2.7 and above and we might
still run tests with 2.6.
On Jun 23, 2015 1:41 AM, "Denis Baryshev" [email protected] wrote:

In st2api/setup.py
#1647 (comment):

+from setuptools import setup, find_packages
+
+
+def fetch_requirements():

  • links = []
  • reqs = []
  • for req in parse_requirements('requirements.txt', session=False):
  •    if req.link:
    
  •        links.append(str(req.link))
    
  •    reqs.append(str(req.req))
    
  • return (reqs, links)

+current_dir = os.path.dirname(os.path.realpath(file))
+version_file = os.path.join(current_dir, '../st2client/st2client/init.py')
+with open(version_file, 'r') as f:

I've added top st2_version file, which is being populated for each
component also if it can be found. This is done by make, so by the type
setup.py is run, all is nice file can be cat.


Reply to this email directly or view it on GitHub
https://github.com/StackStorm/st2/pull/1647/files#r32961529.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In short - for now we still run tests and other lint tasks under Python 2.6 so the easiest / best thing to do is to just replace dict comprehension with dict([list comprehension]).

When we decide to drop support for 2.6 we can re-evaluate that.

vmatch = re.search(r'__version__ = [\'\"](.*)[\'\"]$', f.read(), flags=re.MULTILINE)


install_reqs, dep_links = fetch_requirements()
ST2_COMPONENT = os.path.basename(current_dir)
ST2_VERSION = vmatch.group(1)


setup(
name='st2api',
version='0.4.0',
description='',
name=ST2_COMPONENT,
version=ST2_VERSION,
description='{} component'.format(ST2_COMPONENT),
author='StackStorm',
author_email='[email protected]',
install_requires=[
"pecan",
],
package_data={
'st2api': ['templates/*.html']
},
test_suite='st2api',
install_requires=install_reqs,
dependency_links=dep_links,
test_suite=ST2_COMPONENT,
zip_safe=False,
include_package_data=True,
packages=find_packages(exclude=['ez_setup'])
packages=find_packages(exclude=['setuptools'])
)
6 changes: 6 additions & 0 deletions st2auth/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
eventlet>=0.13.0
oslo.config
passlib>=1.6.2,<1.7
pecan==0.7.0
pymongo<3.0
six==1.9.0
6 changes: 3 additions & 3 deletions st2client/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
prettytable
python-dateutil
pyyaml
jsonpath-rw>=1.3.0
requests
six
python-dateutil
jsonpath-rw
six==1.9.0
4 changes: 4 additions & 0 deletions st2common/MANIFEST.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# https://docs.python.org/2/distutils/sourcedist.html#commands
# Include all files under the source tree by default.
# Other behaviour can be used though.
recursive-include st2common *.* *
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kami, @lakshmi-kannan there was a question why I have removed package_data, it's because the way it works we must check it carefully. Unless we are not sure what files to include better to include all sources and here comes MANIFEST.in..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of just including all in the manifest, I would prefer to explicitly declare all the non-code artifacts we want to include here - so things like README, LICENSE, NOTICE, requirements.txt, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, so far I suggest like this. Later on more comprehensive filtering can
be done.
BTW none code artifacts are included as you might think. Read this as
st2common/{,.*} in bash globing way.
MANIFEST.in is relative to the component root, i.e we include files not
under component root but only under its source folder.

On Thu, Jun 25, 2015, 08:37 Tomaz Muraus [email protected] wrote:

In st2common/MANIFEST.in
#1647 (comment):

@@ -0,0 +1,4 @@
+# https://docs.python.org/2/distutils/sourcedist.html#commands
+# Include all files under the source tree by default.
+# Other behaviour can be used though.
+recursive-include st2common . *

Instead of just including all in the manifest, I would prefer to
explicitly declare all the non-code artifacts we want to include here - so
things like README, LICENSE, NOTICE, requirements.txt, etc.


Reply to this email directly or view it on GitHub
https://github.com/StackStorm/st2/pull/1647/files#r33223754.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still confused by that line - "st2common/{,.*}" basically means everything under st2common/st2common.

In any case, like I said above, I would prefer to be explicit and collect Python modules and packages we want to include inside setup.py and only explicitly declare other non-package files we want to include here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's explicit, all stuff under st2common/st2common. That's package_data works quite implicitly...
I mean you add some files under st2common/st2common/htmls, and setup.py breaks the things since it doesn't include htmls. And so on and so forth.
Including all the source is the most explicit behavior) It's useful now and might be valid in the future.

And package_data - isn't used in 100% of cases, it's limited... and its usage prone to errors, imo. MANIFIST.in is more comprehensive way to go. It's seems not important so far, but I suggest strictly include everything under code and future will introduce new use cases.

3 changes: 3 additions & 0 deletions st2common/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,6 @@ deb:
tar --transform=s~^~$(COMPONENTS)-$(VER)/~ --exclude=correlation -czf ~/$(COMPONENTS).tar.gz bin st2 logrotate.d $(COMPONENTS) ../contrib ../docs ../tools/ ../requirements.txt packaging/debian
pushd ~ && tar -xzf $(COMPONENTS).tar.gz && cd $(COMPONENTS)-$(VER) && cp -Rf packaging/debian ./ && dpkg-buildpackage -us -uc -b && popd
cp -f ~/$(COMPONENT)*.deb ~/debbuild/

.PHONY: requirements
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be in the top level Makefile?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it will be as we progress. [WIP] so far

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And yeah in top level
it will make -s st2*/in-requirements.txt -f fixed-requirements.txt, to merge deps of all of the components.

python ../scripts/fixate-requirements.py -s in-requirements.txt -f ../fixed-requirements.txt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is how it should work in prod, our tooling must refresh requirement.txt, this makefile line does so.
In action: https://gist.github.com/dennybaa/001956639e6e195b3006

15 changes: 15 additions & 0 deletions st2common/in-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
python-dateutil
eventlet>=0.13.0
git+https://github.com/StackStorm/fabric.git@stanley-patched#egg=fabric
jinja2
jsonschema>=2.3.0
kombu
mongoengine>=0.8.7,<0.9
oslo.config
paramiko
pecan==0.7.0
pyyaml
requests
semver>=2.1.2
six==1.9.0
tooz
6 changes: 0 additions & 6 deletions st2common/setup.cfg

This file was deleted.

49 changes: 35 additions & 14 deletions st2common/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,45 @@
# See the License for the specific language governing permissions and
# limitations under the License.

try:
from setuptools import setup, find_packages
except ImportError:
from ez_setup import use_setuptools
use_setuptools()
from setuptools import setup, find_packages
# Just setup pip before using with a command:
# curl https://bootstrap.pypa.io/get-pip.py | python

import os.path
from pip.req import parse_requirements
from setuptools import setup, find_packages


def fetch_requirements():
links = []
reqs = []
for req in parse_requirements('requirements.txt', session=False):
if req.link:
links.append(str(req.link))
reqs.append(str(req.req))
return (reqs, links)

current_dir = os.path.dirname(os.path.realpath(__file__))
with open(os.path.join(current_dir, 'st2_version'), 'r') as f:
st2_version = f.read().strip()

install_reqs, dep_links = fetch_requirements()
st2_component = os.path.basename(current_dir)


setup(
name='st2common',
version='0.4.0',
description='',
name=st2_component,
version=st2_version,
description='{} component'.format(st2_component),
author='StackStorm',
author_email='[email protected]',
install_requires=[
"pecan",
],
test_suite='st2common',
install_requires=install_reqs,
dependency_links=dep_links,
test_suite=st2_component,
zip_safe=False,
include_package_data=True,
packages=find_packages(exclude=['ez_setup'])
packages=find_packages(exclude=['setuptools', 'examples', 'tests']),
scripts=[
'bin/st2-bootstrap-rmq',
'bin/st2-register-content'
]
)
4 changes: 4 additions & 0 deletions st2debug/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
python-gnupg>=0.3.7,<0.4
requests
six==1.9.0
pyyaml
8 changes: 8 additions & 0 deletions st2reactor/requirement.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apscheduler>=3.0.0rc1
python-dateutil
eventlet>=0.13.0
jsonpath-rw>=1.3.0
jsonschema>=2.3.0
kombu
oslo.config
six==1.9.0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed ...s.txt, :))