Next

[dmp42]

This is almost exactly the same as #395 except:

• we now merge a dev branch into master (0.7.0 is released and master is opened again)
• commit (and messages) are clearer, cleaner, fewer, simpler (genuine thanks go to @wking for forcing me into a better engineer to collaborate with :-))
• requirements are regrouped and put inside a folder (cleaner root), following @wking suggestion
• wording for documentation is better, clearer (@wking)
• bumped the package version so that someone running from master clearly reports the next to come version, and not the currently released one

@samalba already LGTM-ed #395 so that should merge relatively fast, but please (@shin- @wking) take a moment to get over this.

And this still fix #377, #376 and #368

[wking]

This is much nicer, thanks :).

On Fri, May 30, 2014 at 08:37:18AM -0700, Mangled Deutz wrote:

• Clearer contribute documentation + authors

Can we just drop the Python dependencies from CONTRIBUTE.md and list
them in requirements/test.txt and maybe
requirements/test-optional.txt? I personally don't have a problem
just dropping them all into requirements/test.txt and having
CONTRIBUTE.md suggest:

$ pip install -r requirements/test.txt

with requirements/test.txt holding:

-r main.txt
nose>=1.3
coverage>=3.7
mock>=1.0
hacking>=0.8

Hacking itself depends on a specific version of flake8 1, so I
wouldn't specify our own dependency.

I'm not sure what our minimal versions actually are. For folks who
like getting locked into explicit versions, I'd add
requirements/frozen.txt with the output of 'pip freeze'. Only
specifying explicit versions in the other requirement files just makes
it less clear which APIs we're actually using. I'm fine pushing this
relaxation off to a subsequent PR though.

• Fix Duplicate settings for which port to bind to for HTTP #368

docker_registry.server.env.defined is a global variable not used
outside the module, so I'd rather call it _SETTINGS (or at least
_DEFINED).

• Entirely trivial cleanup

There's still some non-trivial stuff going on here. How does the
docker-registry-core shift into setup.py ease tox? Why don't we keep
the flake8 config? docker_registry/lib/init.py was empty, so why
the UTF-8 declaration?

• Cleaner, simplified requirements expressions

Do you expect folks to want the style requirements without the test
requirements (test without style makes sense to me)? If not, maybe:

-r main.txt
hacking>=0.8

in style.txt? You'd have to restore the flake8 requirement to
test.txt, with a sufficiently flexible version that we don't get
bitten the next time hacking bumps their excessively constrained
dependency.

• Fix bugsnag requirement #377 && Fix Centralize version string #376

I prefer the nestable $(…) to … for command substitution. But PWD
is a POSIX-specified environment variable 2, so I'd rather have:

pip install --download-cache=~/.pip-cache "file://${PWD}#egg=docker-registry[bugsnag]" || exit 1

• Bring back workflow test

“# XXX revert” ? Does this have some magic effect? If it's aimed at
people, a bit more detail would be nice ;).

• Slightly enhanced driver interface + more verbose exception

I expect the dummy Base.init is to consume **kwargs that are
getting passed down a super() stack. If we expect drivers to be
declaring these particular variables (path and config), notes in the
Base docstring explaining what they are for would be nice. Otherwise
each driver has to check how their particular super-class interprets
them.

[dmp42]

1. please keep hacking separate (in style.txt) - installing it is only useful when you want to flake, and unneeded when you just want the tests, hence slows down tox setup a lot when not needed (and see below)
2. pinned down versions for development packages: this is why CONTRIBUTE list requirements without explicit versions (so that people who are addict to system-wide installs can do whatever pleases them and use whichever version they want and won't work :-)) while the req files are used by automated, isolated testing environments (tox and travis) and have explicit pinned-down requirements (that we know for sure will work and won't break the tests for stupid reasons) - I would really like to keep these pinned-down versions - people who like to test outside of virtualenv and who love to install stuff globally should know what they do, while I would like to encourage others to work in the venv/tox clean rooms. Would be happy to discuss this further though.

docker_registry.server.env.defined is a global variable not used
outside the module, so I'd rather call it _SETTINGS (or at least _DEFINED).

Sure.

There's still some non-trivial stuff going on here.

Sorry if some slipped through - it was not always easy to split the different modifications into coherent commits.

Why don't we keep the flake8 config?

It's useless. Flake ignores it as it found configuration already in the tox file (this (admittedly strange) behavior is documented somewhere on the flake doc site).

docker_registry/lib/init.py was empty, so why the UTF-8 declaration?

So that people who might add something in it don't forget it. Not that useful, agreed - but not that harmful either.

How does the docker-registry-core shift into setup.py ease tox?

Because we want people who pip install docker-registry to get the released version of docker-registry-core, while we want tox tests to be ran against the local version of the core (under depends).
Hence tox would need a separate requirements file listing all the other dependencies, which I think is a burden to maintain and prone to error (that existed earlier, FYI).

Do you expect folks to want the style requirements without the test requirements (test without style makes sense to me)? If not, maybe:

Again, this is about automation, not about what folks install system-wide (and I really think global install is bad practice) - but see above for the reason why I want "style" to stay standalone.

I prefer the nestable $(…) to … for command substitution. But PWD is a POSIX-specified environment variable [2], so I'd rather have:

Agreed.

“# XXX revert” ? Does this have some magic effect?

Yes, it makes differential anisotropic Higgs boson follow the abelian group underneath the unicorn that runs the bike inside the registry :-)

Thanks for the thorough review!

[wking]

On Fri, May 30, 2014 at 09:59:27AM -0700, Mangled Deutz wrote:

the req files are used by automated, isolated testing environments
(tox and travis)

There's no reason you can't have loosely-versioned reqirement files
for humans and explicit frozen files for automation. With just frozen
files you give no guidance to folks using unpinned installs. Will
nose 1.0.0 work? I dunno. Maybe? It's better to just say: “we don't
use anything that's been added to nose since 1.0.0” (or wherever) with
nose>=1.0.0.

There's still some non-trivial stuff going on here.

Sorry if some slipped through - it was not always easy to split the
different modifications into coherent commits.

Understood. And it doesn't have to be perfect to land, otherwise we'd
all just be polishing forever ;). So feel free to say, “that's ok, I
don't care” when I point out some suitably trivial issue.

[explainations of minor stuff]

These make sense to me :). In an ideal world I'd like them written up
in the commit message, but maybe that's over the “too trivial”
threshold.

“# XXX revert” ? Does this have some magic effect?

Yes, it makes differential anisotropic Higgs boson follow the
abelian group underneath the unicorn that runs the bike inside the
registry :-)

:D.

[shin-]

LGTM,

just wondering what purpose docker_registry.server will serve?

[dmp42]

@shin- I would love to cleanup the root of the package - and have under server the things that are pertinent only to the server app itself - lib should still contain things that are also pertinent to maintenance / debugging scripts or other tools accessing the data - and root may only contain entrypoints (or even nothing at all, with the entrypoints in another subpackage).

The point being to have a clearer organization, and possibly to re-expose our scripts more elegantly as cmd entrypoints.

Anyhow, this is rather medium term, as reliability and bugfixes come first.