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

Merge Dogpile.Core into Dogpile.Cache package #91

Closed
sqlalchemy-bot opened this Issue Feb 1, 2016 · 11 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Feb 1, 2016

Migrated issue, originally created by Morgan Fainberg ()

Due to wierdness with python package namespaces (this is a long discussion that invloves pip, system packages, etc) when you have two separate packages providing into the same namespace, there ends up being odd mis-matches.

The plan here is to move dogpile.core into dogpile.cache, and EOL dogpile.core package.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 7, 2016

jvanasco (jvanasco) wrote:

+1 on the general idea, if only for consolidating docs.

BUT i think it would be better to EOL both packages and redistribute them together in a single dogpile package (which would have cache and core directories).

That would preserve the existing namespace, and probably break fewer developer-side integrations. it would also mean that updating existing dogpile.core or dogpile.cache installs would just stop at the last stable version of the current system - and everyone who takes advantage of the new consolidated package would be consciously converting to the new dogpile.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2016

Michael Bayer (zzzeek) wrote:

aesthetically I'm still into the two words "dogpile cache". I guess no matter what we do there's always going to be a dogpile.core install sitting in people's python libs, how do we overcome that ?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2016

Morgan Fainberg () wrote:

Regardless of the case, we likely need to make the consolidated package be incompatible with the older versions and have the new "dogpile.core" do nothing but set requirements.txt to be the new package..

So if we were to standardize on dogpile.cache:

dogpile[.core] would be an empty package with a requirements.txt that says "dogpile.cache >= _new_version_that_rolls_in_dogpile.core".

If we use "dogpile" instead, dogpile.[core|cache] would now be empty and have it's requirements be "dogpile >= new_release".

The old packages would be EOL'd by not seeing any further updates. This will solve the "namespace" issues and get everyone on a consistent install.

I don't really care which direction we go [what the resulting package is], though I have a mild preference to go with dogpile.cache instead of dogpile.core or dogpile.

I am working on rolling up dogpile.core (while preserving commit history) into dogpile.cache and issue a PR so we can see how it looks. I'm a little more than 50% of the way through.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2016

jvanasco (jvanasco) wrote:

My thought was that "dogpile" as a new library consists of .core and .cache as-is.

Both projects would then be EOL via easy_install or pip. There would be 100% compat with existing developer and add-on code. Distributing like this should avoid the packaging issues.

Someone would have to consciously update to "dogpile" though, and in doing so can uninstall the .core and .cache independent versions.

Consolidated into ".cache" would be more seamless, but then you have the core lying around and third party addons or developer code would probably need to update.

Does that make sense?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2016

Morgan Fainberg () wrote:

@jvanasco You still have the namespace issue, dogpile.cache and dogpile.core try and install into "dogpile" in the site-packages. So what happens is you now have 3 packages that are trying to own dogpile and install something into it.

This ends up being a lot of headaches since it now depends on what package installs when/where and could cause conflicts on what is in dogpile/core or dogpile/cache.

The other option would be to drop the "." and move to dogpile_cache, but I think that is a much worse approach in this case.

The basic issue is "python packaging has issues with multiple packages working within a single package [aka filesystem directory] namespace"

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2016

jvanasco (jvanasco) wrote:

Yes and no.

.core and .cache would be EOL so anyone issuing a "upgrade" on a package manager would not be affected.

In order to use the "new" consolidated namespace, someone would need to consciously install "dogpile" -- and would then be told to uninstall the independent versions. The install process could probably also exit with an error message if it detects the .core/.cache libraries.

It's not a perfect solution and would require finessing the setup.py to conflict out earlier versions... But this strategy should allow for the code (and 3rd party libs and implementations) to be relativity unchanged while simplifying the distribution as needed.

users will import things from dogpile.cache to test the null value/miss, and developers will import from dogpile.core -- so changing the package name will break code for some people.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 15, 2016

Morgan Fainberg () wrote:

@jvanasco I think we get the same exact net result: a unified namespace. Is there a real benefit to doing the EOL method you just described? I'm going to go out on a limb and say that forcing an uninstall to upgrade is going to be a lot more painful. If you're doing an upgrade of either today you're still getting new code probably for both (ok so dogpile.core might not force an upgrade of dogpile.cache, but is there a harm in having dogpile.cache code installed when dogpile.core is installed?). I'm 100% fine with the EOL, but I want to make sure we're making the best choices/easiest choices for us, maintainability, and the consumers :)

The original proposal was to keep dogpile.core and dogpile.cache the same as they are today, so 3rd party libs would remain unchainged except an upgrade might result in a different pypi package being installed (distro-packaged such as .deb and .rpm will likely do a conflicts/replaces/whatever-is-the-right term and only offer the new "package").

With what I'm proposing the installed python modules would remain dogpile, dogpile.core, and dogpile.cache. The only thing that would change is where the new code goes in pypi (dogpile.cache vs. split between dogpile.core). You don't need to name the pypi package exactly identical to the module(s) installed on disk. I'd also be ok with just naming it dogpile, and having the new dogpile.cache and dogpile.core pypi packages simply use dogpile in their requirements.txt

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 15, 2016

jvanasco (jvanasco) wrote:

Ah okay. I thought you had wanted the new structure to nest dogpile.core into dogpile.cache;

My primary concern is that the files-on-disk + imports are still:

dogpile = /dogpile
dogpile.core = /dogpile/core/__init__.py
dogpile.cache = /dogpile/cache/__init__.py

beyond that, I was interested in "fixing" potentially broken installs in-the-wild.

correct me if i'm wrong, but your method will only avoid the shared-namespace issues in the future for clean installs. any existing installs will still be affected (and possibly aggravated) by that approach.

the reason why I like forcing an uninstall + new package name (ie. dogpile) to upgrade, is that you noted the conflicts/oddness arises from how multiple packages get installed into a single directory. this strategy would offer the chance to explicitly 'correct' any existing oddnesses for people who need to upgrade (vs only support those on a fresh install).

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 15, 2016

Morgan Fainberg () wrote:

Cool! We are on the same page!

So the way I was addressing the old install -> new, the new dogpile.core would be empty, so nothing in it, just a requirements.txt that goes to the new dogpile package. Same with dogpile.cache. This way upgrade removes the old file structure from disk and replaces with the new dogpile package.

This means we end up with:

dogpile.cache --- Requires --> dogpile (otherwise empty)

dogpile.core --- Requires -> dogpile (otherwise empty)

#!python

  dogpile = /dogpile/__init__.py
            /dogpile/cache/__init__.py
            /dogpile/core/__init__.py

This way imports look exactly the same as you've described.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 3, 2016

Michael Bayer (zzzeek) wrote:

Merge dogpile.core into dogpile.cache.

The dogpile.core package itself is EOLed. The features
within it are now moved underneath the "dogpile"
and "dogpile.util" namespaces, and the namespace packaging
directives are removed. dogpile.cache has no
dependencies on "dogpile.core" as a namespace any longer,
though it does provide dogpile/core.py for backwards
compatibility.

fixes #91

Co-authored-by: Mike Bayer mike_mp@zzzcomputing.com
Change-Id: Ia1ca428616073755aec74c2ac4780cd634092ca8
Pull-request: https://bitbucket.org/zzzeek/dogpile.cache/pull-requests/48

761dc0a

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 3, 2016

Changes by Michael Bayer (zzzeek):

  • changed status to closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment