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

ansible: add jmespath by default #173364

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

stasjok
Copy link
Contributor

@stasjok stasjok commented May 17, 2022

Description of changes

jmespath is needed for a popular jinja filter json_query. It was included by default in ansible_2_9.

We can make it optional, but I think many would like to see it available by default.

Also community.general has filters hashids and jc which also require additional dependencies. Should we include them too?

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@mweinelt
Copy link
Member

mweinelt commented May 17, 2022

To be honest, I'm not quite sure what to do about the ansible package. It should probably offer an extraPackages option, to which python packages can be passed, because we simply can't track all dependencies here and different users will need different things and just one broken dependency will break things for all users.

In my real world daily usage we just install ansible-core and handle the rest via collections in requirements.yml, with additional python dependencies being shipped via a requirements.txt.

@stasjok stasjok force-pushed the ansible-jmespath branch from 83a7d87 to 3e3c30b Compare May 18, 2022 08:50
@ofborg ofborg bot requested review from mweinelt and Mic92 May 18, 2022 09:02
@stasjok
Copy link
Contributor Author

stasjok commented May 18, 2022

I noticed a failed build of deepdiff. It looks like seperman/deepdiff#255. Can I add a package update here or I shouldn't do it? I don't quite sure why ansible-core even depends on it.

It should probably offer an extraPackages option, to which python packages can be passed, because we simply can't track all dependencies here and different users will need different things and just one broken dependency will break things for all users.

I've added extraPackages, but to ansible-core package. This way user can

ansible.override { extraPackages = [ python3Packages.jc ]; }

In my real world daily usage we just install ansible-core and handle the rest via collections in requirements.yml, with additional python dependencies being shipped via a requirements.txt.

I don't see a way to install ansible-core right now with current nixpkgs. python3Packages.ansible package installs only collections (not usable by itself), python3Packages.ansible-core (or just ansible) installs a core with collections. In theory it should be just like upstream: ansible-core installs only core, ansible installs ansible-core which contains "a select group of Collections". In spirit of Nix it would be cool to have something like ansible-core.withPlugins, where a user can provide a list of collections to install (probably just a list of sources fetchFromGithub or even some fetchAnsibleGalaxy fetcher).

@vcunat
Copy link
Member

vcunat commented May 18, 2022

deepdiff was updated as a part of #173429

@stasjok stasjok force-pushed the ansible-jmespath branch from 3e3c30b to 9e27946 Compare May 19, 2022 06:02
@stasjok
Copy link
Contributor Author

stasjok commented May 19, 2022

deepdiff was updated as a part of #173429

Thanks. I removed it from here. But it looks for me that update is broken (hash is not changed).

@vcunat
Copy link
Member

vcunat commented May 19, 2022

extraPackages: by the way, I recently tried a very simple expression for zabbix-api. I could make a PR but overall I'm unsure how to use the stuff. For now I just

mkShell {
  packages = [ ansible zabbix-api ];
}

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 8, 2023
Copy link
Contributor

@dawidd6 dawidd6 left a comment

Choose a reason for hiding this comment

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

Would be great to have this.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 3, 2023
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 7, 2023
@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@mkg20001
Copy link
Member

Rebased because of conflict
@mweinelt should we keep the jmespath addition or just go with the extraPackages option? Since the extraPackages option is already useful on it's own.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 27, 2024
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 28, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 22, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants