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

Incorrect Name #50

Closed
b4ldr opened this issue May 12, 2015 · 6 comments
Closed

Incorrect Name #50

b4ldr opened this issue May 12, 2015 · 6 comments

Comments

@b4ldr
Copy link
Contributor

b4ldr commented May 12, 2015

there is a property of nameserver_count for nscount at
https://github.com/RIPE-NCC/ripe.atlas.sagan/blob/master/ripe/atlas/sagan/dns.py#L57-L60

however this is actually the authoritative section count

@danielquinn
Copy link
Collaborator

I'm a little confused. Are you saying that NSCOUNT as we're getting from the abuf parser is actually the authoritative section count, or that something in the Header class is amiss we just need to rename nscount?

@b4ldr
Copy link
Contributor Author

b4ldr commented May 13, 2015

Well its possible its not a bug the rfc says

NSCOUNT         an unsigned 16 bit integer specifying the number of name
                server resource records in the authority records
                section.

However the authority section can contain more then just name server records e.g. dig mnasd.google.com will have an soa in the auth section. So, yes, nscount is actually the count of authority sections and is very badly named, double check with anand to make sure im not misinterpreting things but thats my understanding

@danielquinn
Copy link
Collaborator

Alright. I'll have a chat with Philip and Anand once the RIPE meeting is over and we'll see what can be done about this.

@danielquinn
Copy link
Collaborator

Alright, I just had a talk with Philip and he can't reproduce the problem. Do you have an example result that we can poke at and reproduce?

@b4ldr
Copy link
Contributor Author

b4ldr commented May 20, 2015

Its not an error that can be reproduced. I just think that nameserver_count would be better renamed as authority_count. however its a really minor thing so feel free to close this

danielquinn added a commit that referenced this issue May 22, 2015
danielquinn added a commit that referenced this issue May 22, 2015
@danielquinn
Copy link
Collaborator

Alright, so I just talked with Philip on this one, and I think we're gonig to keep the name as-is, if for no other reason that it best reflects the original name NSCOUNT. I considered setting up an alias called authority_count, but that would introduce two things that do the same thing for the sake of naming, which doesn't seem cool.

I did however just add a docstring to the nameserver_count for someone who might be poking around the source, and a bit more explanation in the documentation, to hopefully make things a little more obvious.

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

No branches or pull requests

2 participants