Skip to content

yaml filters now use 'ansible yaml' #81727

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

Closed
wants to merge 1 commit into from

Conversation

bcoca
Copy link
Member

@bcoca bcoca commented Sep 19, 2023

Now they can process things like !unsafe and !vault

ISSUE TYPE
  • Feature Pull Request

@ansibot ansibot added feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. labels Sep 19, 2023
@ansibot

This comment was marked as resolved.

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Sep 19, 2023
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Sep 20, 2023
@mattclay mattclay removed the needs_triage Needs a first human triage before being processed. label Sep 21, 2023
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 28, 2023
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Oct 30, 2023
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Oct 30, 2023
@ansibot ansibot removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. ci_verified Changes made in this PR are causing tests to fail. labels Nov 6, 2023
Copy link
Member

@nitzmahone nitzmahone left a comment

Choose a reason for hiding this comment

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

I'm kinda torn about this overall...

In the process of determining which of our serialization formats would support round-tripping datatags, we leaned pretty heavily on the fact that from_yaml didn't support any custom tags today as justification for not defining a YAML tag scheme for datatags (which will be very difficult to do properly, given the limitations of YAML's tagging/type system). Even with this change, to_yaml still only preserves vault, not unsafe.

JSON is really our only "full fidelity round-trip capable" data interchange format today, and I'm thinking we should try to keep it that way. There are so many other gotchas around using Jinja to rewrite Ansible content that it might be good to just continue to let it fail early if someone's trying, rather than letting some stuff through. We'd also continue to fail on any doc that had other undefined tags, and since PyYAML doesn't provide anything out of the box to preserve that stuff, I think it's a lot easier to explain that those filters just don't support tagged data at all rather than defining a special subset that we'd likely face pressure to increase over time.

Doing multi-stream YAML support this way is also problematic, since it's now impossible to tell the difference between a doc with a root list and a list of docs. If we end up using multi-stream and need to support that with the filters, it probably needs to be a custom wrapper type that actually provides the context for each document.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 16, 2023
@bcoca
Copy link
Member Author

bcoca commented Nov 16, 2023

The multidoc support in the filters is preexisting, I just adapted it to be able to use Ansible's YAML as users were hitting an issue when exporting/generating the YAML and not being able to reimport/handle via the filters.

If we don't want to support having the filters understand/handle 'ansible YAML' then i can just turn this into a documentation PR.

@bcoca bcoca marked this pull request as ready for review November 30, 2023 16:32
@bcoca bcoca force-pushed the yaml_filters_use_our_yaml branch from aa0bb98 to 22d8712 Compare December 14, 2023 17:22
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 14, 2023
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Dec 20, 2023
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 2, 2024
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Oct 10, 2024
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jan 28, 2025
@bcoca bcoca force-pushed the yaml_filters_use_our_yaml branch from 22d8712 to b35d74a Compare February 18, 2025 17:11
@ansibot ansibot removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Feb 18, 2025
@ansibot ansibot removed stale_pr This PR has not been pushed to for more than one year. ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Feb 18, 2025
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Feb 19, 2025
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 3, 2025
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Apr 15, 2025
@bcoca bcoca force-pushed the yaml_filters_use_our_yaml branch from b35d74a to d4253ad Compare April 24, 2025 16:02
@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html ci_verified Changes made in this PR are causing tests to fail. labels Apr 24, 2025
@ansibot
Copy link
Contributor

ansibot commented Apr 24, 2025

The test ansible-test sanity --test import --python 3.11 [explain] failed with 1 error:

lib/ansible/plugins/filter/core.py:36:0: traceback: ImportError: cannot import name 'from_yaml' from 'ansible.parsing.yaml' (/root/ansible/lib/ansible/parsing/yaml/__init__.py)

The test ansible-test sanity --test import --python 3.12 [explain] failed with 1 error:

lib/ansible/plugins/filter/core.py:36:0: traceback: ImportError: cannot import name 'from_yaml' from 'ansible.parsing.yaml' (/root/ansible/lib/ansible/parsing/yaml/__init__.py)

The test ansible-test sanity --test import --python 3.13 [explain] failed with 1 error:

lib/ansible/plugins/filter/core.py:36:0: traceback: ImportError: cannot import name 'from_yaml' from 'ansible.parsing.yaml' (/root/ansible/lib/ansible/parsing/yaml/__init__.py)

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -l --json -t filter ansible.builtin" returned exit status 0.
>>> Standard Error
[WARNING]: Skipping file b'/root/ansible/lib/ansible/plugins/filter/core.py': "Failed to load b'/root/ansible/lib/ansible/plugins/filter/core.py' for ansible.builtin: cannot import name 'from_yaml' from 'ansible.parsing.yaml' (/root/ansible/lib/ansible/parsing/yaml/__init__.py)"
[WARNING]: Skipping plugin (/root/ansible/lib/ansible/plugins/filter/core.py), cannot load: cannot import name 'from_yaml' from 'ansible.parsing.yaml' (/root/ansible/lib/ansible/parsing/yaml/__init__.py)

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/plugins/filter/core.py:35:0: unused-import: Unused yaml_load imported from ansible.module_utils.common.yaml

click here for bot help

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 24, 2025
@bcoca bcoca closed this Apr 24, 2025
@bcoca bcoca deleted the yaml_filters_use_our_yaml branch April 24, 2025 16:47
@ansible ansible locked and limited conversation to collaborators May 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
data_tagging feature This issue/PR relates to a feature request. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants