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

keep_keys: fix keeping all lists, even wrong ones #379

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gaelgatelement
Copy link

SUMMARY

keep_keys since #370 was keeping all lists items, even if there's no match in the key of the list.

ISSUE TYPE
COMPONENT NAME

keep_keys

Copy link
Contributor

@gaelgatelement gaelgatelement force-pushed the gaelg/fix-keep-keys-keeping-wrong-lists branch from 64d24a7 to 09432ea Compare October 8, 2024 07:12
Copy link
Contributor

@MPStudyly
Copy link

Any progress on this fix? The previous change just blew up our play, as we heavily rely on keep_keys to work as expected. Seeing how breaking code was merged within a few days and the fix lying around over a month is not very promising :/

@roverflow
Copy link
Member

roverflow commented Nov 6, 2024

@gaelgatelement @MPStudyly We are currently evaluating this. Initially, we had to make a tough decision to implement this change, as there are cases where a greedy approach to retaining keys is necessary. We are now planning to introduce an additional attribute, which will serve as a toggle to enable a non-greedy approach. This should serve all the usecases that are out there. If @gaelgatelement is open to making these adjustments in this PR, we’d greatly appreciate it; otherwise, it may take us some time to release those changes from our end.

@MPStudyly
Copy link

@gaelgatelement @roverflow any update on this? We're still stuck on v5.1.0 of this collection due to this change in behavior :/

@gaelgatelement
Copy link
Author

Unfortunately I'm now lacking the time to implement the non-greedy version. Surely it's about keeping both keep_keys methods and adding a flag to use one or the other?

@MPStudyly
Copy link

Unfortunately I'm now lacking the time to implement the non-greedy version.

Sad to hear :(

Surely it's about keeping both keep_keys methods and adding a flag to use one or the other?

Exactly, we rely on the old behavior to filter data from a structure "dictated" by a role we are relying on. This was conveniently doable with the previous behavior of keep_keys, but isn't anymore as of now.
Besides hoping for soon introduction of the behavior restoring flag, we're actively working in untangling our implementation to hopefully be able to omit this filter all together. The latter is not necessarily done due to the change in its behavior, but because it often is unclear where what info got removed when vars are resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants