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

Fix naive fileserver map diff algorithm #33875

Merged
merged 1 commit into from
Jul 27, 2016

Conversation

jmesquita
Copy link

@jmesquita jmesquita commented Jun 8, 2016

What does this PR do?

Fixes fileserver map diffing algorithm

Previous Behavior

Fired fileserver event would say changed is False in case a file was modified and none was addded/removed.

New Behavior

Changed flag is now correct on the event

Tests written?

Yes

Naively comparing sorted dict keys does not guarantee that maps are equal. We
must compare mtimes for filenames in case keys are the same to make sure there
isnt a modified file.

Naively comparing sorted dict keys does not guarantee that maps are equal. We
must compare mtimes for filenames in case keys are the same to make sure there
isnt a modified file.
@cachedout
Copy link
Contributor

Go Go Jenkins!

1 similar comment
@rallytime
Copy link
Contributor

Go Go Jenkins!

@cachedout
Copy link
Contributor

@terminalmage Could you take a look at this?

:codeauthor: :email:`Joao Mesquita <jmesquita@sangoma.com>`
'''

# Import pytohn libs
Copy link
Contributor

Choose a reason for hiding this comment

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

Python

@terminalmage terminalmage added ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch. bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch and removed ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch. labels Jun 29, 2016
@terminalmage
Copy link
Contributor

This needs to be backported to 2015.8 once fixes have been made.

@rallytime
Copy link
Contributor

Just a bump here @jmesquita - we'd love to get this in once you've addressed the comments above.

@cachedout
Copy link
Contributor

@jmesquita Will you be able to get back to this to make the requested changes?

@cachedout
Copy link
Contributor

I think we can make these fixes after the merge. I'm going to get this in and then fix it up.

@cachedout cachedout merged commit 7956dbe into saltstack:develop Jul 27, 2016
cachedout pushed a commit to cachedout/salt that referenced this pull request Jul 27, 2016
cachedout pushed a commit that referenced this pull request Jul 27, 2016
@cachedout
Copy link
Contributor

#34999 will need to be backported as well.

@rallytime rallytime added ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch. and removed bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch labels Jul 27, 2016
rallytime pushed a commit to rallytime/salt that referenced this pull request Jul 27, 2016
rallytime pushed a commit that referenced this pull request Jul 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants