Skip to content
This repository has been archived by the owner on Jul 16, 2024. It is now read-only.

Guaranteed order in deduplication dictionary hashes #98

Merged
merged 8 commits into from
Dec 28, 2018
Merged

Guaranteed order in deduplication dictionary hashes #98

merged 8 commits into from
Dec 28, 2018

Conversation

CameronJHall
Copy link

@CameronJHall CameronJHall commented Dec 6, 2018

GREASE Developer Pull Request Checklist Guaranteed order in deduplication dictionary hashes

USE THIS TEMPLATE FOR YOUR PULL REQUEST!

Purpose

In deduplication, the order of items in the dictionary to be hashed is not guaranteed. This PR fixes that by turning an unsorted dictionary into an ordered tuple before hashing.

Expected Outcome

Hashes will be consistent regardless of the ordering of the incoming dictionary in deduplication.

Checklist

  • Tests
  • Documentation
  • Maintainer Code Review

Cameron Hall added 4 commits November 29, 2018 16:12
Signed-off-by: Cameron Hall <[email protected]>
Signed-off-by: Cameron Hall <[email protected]>
Signed-off-by: Cameron Hall <[email protected]>
Signed-off-by: Cameron Hall <[email protected]>
@CameronJHall CameronJHall changed the title Guaranteed order in deduplication dicitionary hashes Guaranteed order in deduplication dictionary hashes Dec 6, 2018
Copy link
Collaborator

@abstract-base-method abstract-base-method left a comment

Choose a reason for hiding this comment

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

Wow! Great work @cjhall1283 !!

  1. Please remove the print statement I referenced. I suppose it was left behind in local development
  2. Please increment the minor version number. Your release will be 2.5.0

tgt_grease/enterprise/Model/DeDuplication.py Outdated Show resolved Hide resolved
@abstract-base-method abstract-base-method self-assigned this Dec 7, 2018
@abstract-base-method abstract-base-method added enhancement awating contributor outstanding action required from contributor labels Dec 7, 2018
@CameronJHall
Copy link
Author

It's also worth noting that this only effects T1 deduplication so there is almost no cost to migrate from 2.4.x to 2.5.0.

Copy link
Collaborator

@abstract-base-method abstract-base-method left a comment

Choose a reason for hiding this comment

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

Nice work here!

Signed-off-by: Cameron Hall <[email protected]>
@CameronJHall
Copy link
Author

using $addToSet instead of $push when adding prototypes to roles in mongo prevents multiple of the same prototype getting logged even if there is only a singe instance on the grease execution server itself.

Copy link
Collaborator

@abstract-base-method abstract-base-method left a comment

Choose a reason for hiding this comment

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

I really like the work done here. Excellent job!

@abstract-base-method abstract-base-method merged commit 2085e2a into target:master Dec 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awating contributor outstanding action required from contributor enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants