Skip to content

xattr module updates - #12048

Draft
bcoca wants to merge 3 commits into
ansible-collections:mainfrom
bcoca:xattr_up
Draft

xattr module updates#12048
bcoca wants to merge 3 commits into
ansible-collections:mainfrom
bcoca:xattr_up

Conversation

@bcoca

@bcoca bcoca commented May 14, 2026

Copy link
Copy Markdown
Contributor
SUMMARY
  • now uses system calls instead of shelling out this should make it more usable (no CLI needed), less fragile (no scrape!) and speed it up (no shell/fork)
  • moved to case statement
  • ensure we have message and result in all cases

TODO:

  • changelog
  • tests
  • update docs and add return
ISSUE TYPE
  • Refactoring Pull Request
COMPONENT NAME

module xattr

ADDITIONAL INFORMATION

@ansibullbot ansibullbot added WIP Work in progress module module plugins plugin (any type) labels May 14, 2026
@ansibullbot

This comment was marked as outdated.

Sign in to view
@felixfontein felixfontein added the check-before-release PR will be looked at again shortly before release and merged if possible. label May 14, 2026
@felixfontein

felixfontein commented May 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Note that case isn't supported in Python 3.8, which ansible-core 2.18 still supports. It will take another year until we can use that in c.g for code that runs on the target :)

Edit: ah, wait - more like 2.5 years, when we stop supporting ansible-core 2.21...

@ansibullbot

This comment was marked as outdated.

Sign in to view
russoz
russoz reviewed May 17, 2026
View reviewed changes

@russoz russoz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @bcoca Thanks for the improvement!

Got a couple of comments there, there's obviously the changelog frag, and a comment for future PRs: states read, all, keys should be transposed to a new module xattr_info and deprecated from here.

Comment thread plugins/modules/xattr.py Outdated
Comment thread plugins/modules/xattr.py Outdated
Comment thread plugins/modules/xattr.py Outdated
@ansibullbot

This comment was marked as outdated.

Sign in to view
@ansibullbot

This comment was marked as outdated.

Sign in to view
russoz
russoz reviewed May 17, 2026
View reviewed changes
Comment thread plugins/modules/xattr.py
type: bool
default: true
notes:
- Starting with community.general 13.0.0 this action no longer requires the xattr CLI tools installed on the target.

@russoz russoz May 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

13.0.0 is being released in less than 24h - this may need adjusting if we merge after that.
Unless @felixfontein wants to wait for it.

@bcoca bcoca May 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

instead of continually updating it, lets just do so when out of draft and ready to merge

@felixfontein felixfontein added the backport-13 Automatically create a backport for the stable-13 branch label May 18, 2026
@bcoca @russoz
xattr module updates
e0a570f 
 - now uses system calls instead of shelling out
   this should make it more usable (no CLI needed),
   less fragile (no scrape!) and speed it up (no shell/fork)
 - moved to case statement
 - ensure we have message and result in all cases
 - state read w/o key now issues a warning, it works as state == all

 TODO:
  - changelog
  - tests

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
@ansibullbot

This comment was marked as outdated.

Sign in to view
russoz
russoz reviewed May 21, 2026
View reviewed changes

@russoz russoz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey @bcoca

Once again, thanks working on this. This LGTM as far as I can tell - not using xattrs a lot lately.

@felixfontein

felixfontein commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Right now the tests are failing, so I guess it's not LGTM yet :)

@russoz

russoz commented May 21, 2026

Copy link
Copy Markdown
Collaborator

"LGTM" ;-)

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@russoz russoz russoz left review comments

Assignees

No one assigned

Labels

backport-13 Automatically create a backport for the stable-13 branch check-before-release PR will be looked at again shortly before release and merged if possible. module module plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging WIP Work in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bcoca @ansibullbot @felixfontein @russoz