Setuptools packaging improvments #1647

dennybaa · 2015-06-17T23:05:29Z

Setup.py is not used and not working now as it should. Basically, we devs, just need pip, virtualenv and requirements.txt. However it's interesting that setup.py is really useful when it comes to packaging and distribution.

I've started to work on clearing up st2 dependency and packaging problems. There are a few ideas I suggest to take into work.

We keep on using requirements.txt and *-requirements.txt as the source of dependency truth.
We must include dependencies into the specific component requirements.txt file, as we decided to distribute each component separately (this is consequence of our choice).
We keep using all-in-one requirements.txt and *-requirements.txt for the current workflow compatibility. But basically it should be kept, imo, just because it's handy to setup all deps for st2 on dev machine...

As the result and after proper setup.py fixes, component packages should be able to be built and installed with setup.py. This will allow us packaging and distributing st2 components using pypi as well as debs, rpms etc since majority of tools rely on the python native setup.py method.

PS. I'd better use naming dev-requirements.txt rather than test-*, imo.

Kami · 2015-06-20T09:17:33Z

I will have a look shortly.

In general I agree that we need a working setup.py, especially since we eventually plan do distribute components as wheels.

Kami · 2015-06-20T09:21:19Z

st2api/setup.py

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


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.

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.

Kami · 2015-06-20T09:24:40Z

If we keep and have two requirements file which are managed manually (global and one per component), things will be hard to maintain and get out of sync.

We can update Makefile to install dependencies from each component and maybe also add a new target which combines all the requirements in a global requirements file.

Kami · 2015-06-20T09:26:29Z

Overall, nice work 👍

We also need to make sure we add a new integration test which tries to install each component (e.g. python setup.py install and / or using pip), otherwise I'm pretty sure things will get out of sync and break soon.

Kami · 2015-06-22T11:44:40Z

st2api/setup.py

+
+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:


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).

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.

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.

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.

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.

dennybaa · 2015-06-22T13:37:19Z

That's a nice idea to create a Makefile, so that we manage only
component specific requirement's. Right?

On Sat, Jun 20, 2015 at 12:24 PM, Tomaz Muraus
[email protected] wrote:

If we keep and have two requirements file which are managed manually
(global and one per component), things will be hard to maintain and
get out of sync.

We can update Makefile to install dependencies from each component
and maybe also add a new target which combines all the requirements
in a global requirements file.

—
Reply to this email directly or view it on GitHub.

dennybaa · 2015-06-22T13:40:44Z

Thanks)

On Sat, Jun 20, 2015 at 12:26 PM, Tomaz Muraus
[email protected] wrote:

Overall, nice work

We also need to make sure we add a new integration test which tries
to install each component (e.g. python setup.py install and / or
using pip), otherwise I'm pretty sure things will get out of sync and
break soon.

—
Reply to this email directly or view it on GitHub.

dennybaa · 2015-06-22T13:48:41Z

Don't actually merge so far) This is just an idea. If you like it I can keep this up to date with my work.
I've actually started this, because:

I want specific requirements.txt for any component and just to keep it clear and sane.
I want to build solid stand-alone debian packages (without post-inst hackary)

lakshmi-kannan · 2015-06-23T04:53:48Z

requirements.txt

@@ -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.

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.

lakshmi-kannan · 2015-06-23T05:02:08Z

Overall, seems to be proceeding in right direction but I am not a setup tools expert. I am learning as well.

Kami · 2015-06-24T05:17:53Z

scripts/fixate-requirements.py

+from distutils.version import StrictVersion
+
+
+cwd = os.path.abspath(os.getcwd())


Instead of using cwd, it would be safer to use location where this file is located and work from there.

BASE_DIR = os.path.dirname(os.path.abspath(__file__)) ...

Kami · 2015-06-24T05:34:34Z

Can you please also sync your branch with latest master?

I've added some changes so we not also lint Python files in scripts/ directory.

dennybaa · 2015-06-24T17:35:39Z

st2common/MANIFEST.in

+# 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 *.* *


@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..

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.

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.

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.

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.

dennybaa · 2015-06-25T06:47:09Z

I've taken this from travis

# Merge into one st2 components-wide requirements.txt file.
python ./scripts/fixate-requirements.py -s st2*/in-requirements.txt -f fixed-requirements.txt
  File "./scripts/fixate-requirements.py", line 110
    fixedreq_hash = {req.req.project_name: req for req in fixed if req.req}
                                                 ^
SyntaxError: invalid syntax
make: *** [requirements] Error 1

Maybe I don't get smth or missing, it looks like a valid dict comprehension, moreover I've run this code. @Kami could you give a clue.

Kami · 2015-06-25T09:21:24Z

Another thing I just remembered - we should also ship tests with each component (tests/ sub-directory) and those tests should be runnable.

Most of the tests depend on st2tests package which means we also need to ship this package with each component otherwise user won't be able to run component specific tests.

Kami · 2015-06-25T09:23:25Z

Please let me know when all the comments have been addressed and I will go, check out the branch and test the changes locally.

In addition to that, I will probably also make some small changes (add a readme to each component, make sure we include license and notice file with each component, etc.).

dennybaa · 2015-06-26T10:29:02Z

@Kami, regarding st2tests.
We should not ship tests with packages, it's not needed. This kind of tests should better stay on dev boxes and not deployed to production. It's an overkill, that's why it's not needed.
Production tests may exist though, but it's a different story from unit testing and bdd testing.

Kami · 2015-06-30T07:38:48Z

Closing in favor of #1668.

dennybaa added 2 commits June 17, 2015 21:36

added requirements.txt for specific components

036177d

setup.py new requirements idea

93a1baa

Kami reviewed Jun 20, 2015
View reviewed changes

Kami reviewed Jun 22, 2015
View reviewed changes

dennybaa added 2 commits June 22, 2015 20:24

fixate-requirements.py

159d797

import cleanups

4d973be

lakshmi-kannan reviewed Jun 23, 2015
View reviewed changes

Kami reviewed Jun 24, 2015
View reviewed changes

dennybaa added 2 commits June 24, 2015 16:06

Merge branch 'master' of github.com:StackStorm/st2 into setuptools-fixes

d3dec45

requirements move on, source merging with fixate-requirements.py

48a07a6

dennybaa reviewed Jun 24, 2015
View reviewed changes

renaming requirements.txt -> in-requirements.txt, makefiles fixes

a51c4f2

removing versions from in-requirements.txt

68b0826

dennybaa added 2 commits June 26, 2015 13:44

setup.py unify

09593f8

version bump

d43f06d

Kami mentioned this pull request Jun 29, 2015

Setuptools packaging improvements - make each st2 component packagable and installable via package #1668

Merged

3 tasks

Kami changed the title ~~[WIP] Setuptools packaging improvments~~ Setuptools packaging improvments Jun 30, 2015

Kami closed this Jun 30, 2015

This was referenced Jun 30, 2015

[WIP] Setuptools packaging improvements - make each st2 component packagable and installable via package #1673

Merged

[WIP] Setuptools packaging improvements - make each st2 component packagable and installable via package #1677

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setuptools packaging improvments #1647

Setuptools packaging improvments #1647

dennybaa commented Jun 17, 2015

Kami commented Jun 20, 2015

Kami Jun 20, 2015

Kami Jun 20, 2015

Kami commented Jun 20, 2015

Kami commented Jun 20, 2015

Kami Jun 22, 2015

dennybaa Jun 22, 2015

dennybaa Jun 22, 2015

Kami Jun 25, 2015

Kami Jun 25, 2015

dennybaa commented Jun 22, 2015

dennybaa commented Jun 22, 2015

dennybaa commented Jun 22, 2015

lakshmi-kannan Jun 23, 2015

dennybaa Jun 23, 2015

lakshmi-kannan commented Jun 23, 2015

Kami Jun 24, 2015

Kami commented Jun 24, 2015

dennybaa Jun 24, 2015

Kami Jun 25, 2015

dennybaa Jun 25, 2015

Kami Jun 25, 2015

dennybaa Jun 25, 2015

dennybaa commented Jun 25, 2015

Kami commented Jun 25, 2015

Kami commented Jun 25, 2015

dennybaa commented Jun 26, 2015

Kami commented Jun 30, 2015

		from setuptools import setup, find_packages


		def fetch_requirements():

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

		from distutils.version import StrictVersion


		cwd = os.path.abspath(os.getcwd())

Setuptools packaging improvments #1647

Setuptools packaging improvments #1647

Conversation

dennybaa commented Jun 17, 2015

Kami commented Jun 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kami commented Jun 20, 2015

Kami commented Jun 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dennybaa commented Jun 22, 2015

dennybaa commented Jun 22, 2015

dennybaa commented Jun 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lakshmi-kannan commented Jun 23, 2015

Choose a reason for hiding this comment

Kami commented Jun 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dennybaa commented Jun 25, 2015

Kami commented Jun 25, 2015

Kami commented Jun 25, 2015

dennybaa commented Jun 26, 2015

Kami commented Jun 30, 2015