Skip to content

Fix map comparison of nil values and missing keys#108

Merged
evanphx merged 1 commit intoevanphx:masterfrom
liggitt:map-compare
Aug 8, 2020
Merged

Fix map comparison of nil values and missing keys#108
evanphx merged 1 commit intoevanphx:masterfrom
liggitt:map-compare

Conversation

@liggitt
Copy link
Copy Markdown
Contributor

@liggitt liggitt commented Jun 24, 2020

The existing matchesValue map comparison treats missing keys and nil values equivalently, which means it can return true for maps that are not actually equal

cc @logicalhan

Copy link
Copy Markdown

@logicalhan logicalhan left a comment

Choose a reason for hiding this comment

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

/lgtm

@logicalhan
Copy link
Copy Markdown

/assign @evanphx

liggitt added a commit to liggitt/kubernetes that referenced this pull request Jun 24, 2020
@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented Jun 25, 2020

fyi - green CI in kubernetes with this fix at kubernetes/kubernetes#92482

@logicalhan
Copy link
Copy Markdown

@evanphx would you mind looking over this when you have a chance?

@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented Jul 25, 2020

@evanphx friendly ping

lblackstone added a commit to pulumi/pulumi-kubernetes that referenced this pull request Jul 31, 2020
A performance bug was discovered that can cause CRD
diffs to fail. This bug is fixed in an open PR [1] to the upstream
repo, but has not been merged yet. In the meantime, I forked
the json-patch repo, and applied the fix [2]. This change will be
reverted once the upstream patch merges, and the module will
be updated to point at the fixed changeset.

[1] evanphx/json-patch#108
[2] https://github.com/pulumi/json-patch
lblackstone added a commit to pulumi/pulumi-kubernetes that referenced this pull request Aug 1, 2020
* Update json-patch mod to fix hangs on pulumi update

A performance bug was discovered that can cause CRD
diffs to fail. This bug is fixed in an open PR [1] to the upstream
repo, but has not been merged yet. In the meantime, I forked
the json-patch repo, and applied the fix [2]. This change will be
reverted once the upstream patch merges, and the module will
be updated to point at the fixed changeset.

[1] evanphx/json-patch#108
[2] https://github.com/pulumi/json-patch

* gofmt
@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented Aug 5, 2020

if this and #111 were merged, and a v4.8.0 release cut, 4.x consumers could pick up performance and panic bugfixes on the 4.x stream without getting breaking behavior changes, which would be great

@evanphx evanphx merged commit 162e562 into evanphx:master Aug 8, 2020
@evanphx
Copy link
Copy Markdown
Owner

evanphx commented Aug 8, 2020

So sorry about the delay! @liggitt this at #111 are merged and there is a v4.8.0 tagged against master.

Please let me know if there are still issues and we'll get them sorted quickly.

Again, apologies about the delay.

@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented Aug 8, 2020

thank you very much, this gives us a level we can use on release branches

@liggitt liggitt deleted the map-compare branch August 8, 2020 14:12
@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented Aug 8, 2020

Please let me know if there are still issues and we'll get them sorted quickly.

I did open #113 to make 4.x releases addressable by tag again, which is the last issue I'm aware of with the 4.x stream

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