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

auth: Make sure that we use strict weak records ordering in the API #6816

Merged
merged 1 commit into from Aug 1, 2018

Conversation

Projects
None yet
3 participants
@rgacogne
Member

rgacogne commented Jul 31, 2018

Short description

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
@zeha

This comment has been minimized.

Show comment
Hide comment
@zeha

zeha Jul 31, 2018

Collaborator

Please (manually) check that this doesn't break returning records+comments when there is an RRset with no records but comments left...

Apart from that LGTM

Collaborator

zeha commented Jul 31, 2018

Please (manually) check that this doesn't break returning records+comments when there is an RRset with no records but comments left...

Apart from that LGTM

@rgacogne

This comment has been minimized.

Show comment
Hide comment
@rgacogne

rgacogne Jul 31, 2018

Member

Please (manually) check that this doesn't break returning records+comments when there is an RRset with no records but comments left...

Nice catch, I reverted the ordering of records and comments in fillZone() to the existing behavior, and I added a test detecting a mismatch between records and comments.

Member

rgacogne commented Jul 31, 2018

Please (manually) check that this doesn't break returning records+comments when there is an RRset with no records but comments left...

Nice catch, I reverted the ordering of records and comments in fillZone() to the existing behavior, and I added a test detecting a mismatch between records and comments.

@zeha

zeha approved these changes Jul 31, 2018

@rgacogne rgacogne merged commit b5b468c into PowerDNS:master Aug 1, 2018

4 checks passed

LGTM analysis: C/C++ No alert changes
Details
LGTM analysis: JavaScript No alert changes
Details
LGTM analysis: Python No alert changes
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rgacogne rgacogne deleted the rgacogne:auth-api-strict-ordering branch Aug 1, 2018

rgacogne added a commit that referenced this pull request Aug 2, 2018

Merge pull request #6817 from rgacogne/auth41-api-strict-ordering
auth-4.1.x: Backport #6816: Make sure that we use strict weak records ordering in the API
@rgacogne

This comment has been minimized.

Show comment
Hide comment
@rgacogne

rgacogne Aug 29, 2018

Member

For the record, this solves an issue reported to the mailing-list: https://mailman.powerdns.com/pipermail/pdns-users/2018-July/025420.html

Member

rgacogne commented Aug 29, 2018

For the record, this solves an issue reported to the mailing-list: https://mailman.powerdns.com/pipermail/pdns-users/2018-July/025420.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment