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

parallel-full: init SQL and CSV support #77619

Merged
merged 1 commit into from Apr 3, 2020

Conversation

tomberek
Copy link
Contributor

Motivation for this change

Allow using parallel's SQL features. --sqlmaster and --sqlworker

Things done

Created parallel-extra to prevent adding dependencies to existing users.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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): increased by 14.7M to add SQL support (93.2M -> 107.9M)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@erikarvstedt
Copy link
Member

Does parallel-extra enable all extra features? Then parallel-full is a better name.

@tomberek
Copy link
Contributor Author

Added CSV support and changed name to parallel-full. I suppose we could add every database this could support, but that's a bit too much.

@tomberek tomberek changed the title parallel: init SQL support parallel: init SQL and CSV support Jan 29, 2020
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/132

wrapProgram $out/bin/parallel \
--prefix PATH : "${stdenv.lib.makeBinPath [ procps perl ]}"
--prefix PATH : "${stdenv.lib.makeBinPath [ procps perl ]}" \
--set PERL5LIB "${perlPackages.makeFullPerlPath extraPerlPackages}"
Copy link
Member

Choose a reason for hiding this comment

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

Since it is just an environment variable added I would make parallel-full a wrapper around parallel that adds these packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not 100% sure if the wrapper approach i used is correct, but it works. Open to a better approach.

parallel: add pacakges for CSV function
parallel: use wrapper to include more libraries
parallel: remove unneeded with
parallel: remove args

Co-Authored-By: Jörg Thalheim <Mic92@users.noreply.github.com>
parallel: move default pacakges for full into wrapper
@Mic92 Mic92 changed the title parallel: init SQL and CSV support parallel-full: init SQL and CSV support Apr 3, 2020
@Mic92 Mic92 merged commit 939976a into NixOS:master Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants