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.

Sign up for GitHub

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

Jump to bottom

Remove pythonPackages.flask_cache #43693

Merged
merged 2 commits into from Jul 19, 2018
Merged

Remove pythonPackages.flask_cache #43693

merged 2 commits into from Jul 19, 2018

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Jul 17, 2018 •
edited

Motivation for this change

  • See the first commit (d3d1b88) for the original motivation

  • The second commit moves the graphite_api expression (which was changed previously) into its own file.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@Ma27
pythonPackages.flask_cache: remove
d3d1b88 
The package is currently broken and most folks recommend to use
`flask-caching` (see
thadeusb/flask-cache#171).

The only package in `nixpkgs` which remained to use `flask_cache`
(`graphite_api`) still builds with `pythonPackages.flask-caching`.

The removal and corresponding `graphite_api` change unbreaks several
Hydra builds (see https://hydra.nixos.org/build/76953777).
@Ma27 Ma27 requested a review from FRidh as a code owner July 17, 2018 21:09
@dotlambda
Copy link
Member

@GrahamcOfBorg build python2.pkgs.graphite_api python3.pkgs.graphite_api

dotlambda
dotlambda reviewed Jul 17, 2018
View reviewed changes
pkgs/development/python-modules/graphite-api/default.nix Outdated
}:

buildPythonPackage rec {
name = "graphite-api-1.0.1";
Copy link
Member

Choose a reason for hiding this comment

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

pname, version

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: python2.pkgs.graphite_api, python3.pkgs.graphite_api

Partial log (click to expand)

Ran 119 tests in 9.671s

OK
......................
----------------------------------------------------------------------
Ran 119 tests in 10.035s

OK
/nix/store/5f09jg0fhfv9im7yawr8g4wqdhznh9b1-python2.7-graphite-api-1.0.1
/nix/store/3ww65q2826pl1bbhqg96nqj52f9mbl53-python3.6-graphite-api-1.0.1

@dotlambda
Copy link
Member

Even if this builds, I'm not sure it will work without applying brutasse/graphite-api@842e93f.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: python2.pkgs.graphite_api, python3.pkgs.graphite_api

Partial log (click to expand)

{"index_path": "/tmp/graphite-api-index", "total_entries": 0, "duration": 6.699562072753906e-05, "event": "search index reloaded"}
{"timezone": "UTC", "event": "configured timezone"}
.................................................................................................Fontconfig error: Cannot load default config file
......................
----------------------------------------------------------------------
Ran 119 tests in 2.716s

OK
/nix/store/62qirzzd1rnwnyy1smff29ff5lr47k6d-python2.7-graphite-api-1.0.1
/nix/store/3ym18y8z2vn19dk4kb7ns6k044zjc92f-python3.6-graphite-api-1.0.1

@dotlambda dotlambda mentioned this pull request Jul 17, 2018
Closed
@Ma27
Copy link
Member Author

Ma27 commented Jul 17, 2018

thanks for your feedback @dotlambda!

I fixed the pname/version vs name issue and applied the patch you've linked to 1.3.1 of graphite_api using git apply -3 (it didn't apply from scratch, so I couldn't use fetchpatch).

Is there anything else to fix? :)

dotlambda
dotlambda reviewed Jul 18, 2018
View reviewed changes
pkgs/development/python-modules/graphite-api/default.nix Outdated
sha256 = "0sz3kav2024ms2z4q03pigcf080gsr5v774z9bp3zw29k2p47ass";
};

patches = [ ./flask-caching-rebased.patch ];
Copy link
Member

Choose a reason for hiding this comment

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

Please add a link to brutasse/graphite-api#239.

@Ma27
pythonPackages.graphite_api: move expression
1e454c5 
According to the current convention python packages should live in
`pkgs/development/python-modules`. As I altered the `graphite_api`
expression previously in d3d1b88 I decided to move it as well.

Additionally I applied some minor refactorings:

* use `fetchFromGitHub` instead of `fetchgit`.

* use `checkInputs` for test dependencies

* got rid of fixed points

* applied patch which supports flask-caching to 1.3.1 using `git apply
  -3`
@Ma27
Copy link
Member Author

Ma27 commented Jul 18, 2018

@dotlambda done %)

@dotlambda dotlambda merged commit 8ab563d into NixOS:master Jul 19, 2018
@dotlambda
Copy link
Member

@Ma27 Btw, are you actually using the graphite_api package?

@Ma27 Ma27 deleted the remove-flask_cache branch July 19, 2018 11:46
@Ma27
Copy link
Member Author

Ma27 commented Jul 19, 2018

long story short, I wanted to try out flask_cache, found flask_caching (and it's outdated and broken), so I decided to drop it from my package set and contribute it back to upstream nixpkgs.
graphite_api was just a "fallout" as it depended on flask_cache, that's all :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda left review comments

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: clean-up 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Ma27 @dotlambda @GrahamcOfBorg