Opened 8 years ago

Closed 8 years ago

#1427 closed task (complete)

Socket cache

Reported by: vorner Owned by: vorner
Priority: medium Milestone: Sprint-20111220
Component: ~Boss of BIND (obsolete) Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket: Socket creator
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

This is subtask of #802 and the cache for sockets should be created in here. The interface for the cache is in the trac802 branch in src/lib/python/isc/bind10/socket_cache.py. Further information about how it will be used is to be found in #802 ticket and src/bin/bind10/creatorapi.txt.

Subtickets

Change History (7)

comment:1 Changed 8 years ago by vorner

  • Owner set to vorner
  • Status changed from new to accepted

comment:2 Changed 8 years ago by vorner

  • Owner changed from vorner to UnAssigned
  • Status changed from accepted to reviewing

This one is ready for review. The structures indexing it by the application, token and address look slightly complicated, but it shouldn't be anything extra hard.

It is based on top of the trac802 branch, so the review should be from 51f3cb54492ef02e4951afb15a9c40ba0cdff4ce.

Thanks

comment:3 Changed 8 years ago by jelte

  • Owner changed from UnAssigned to jelte

comment:4 follow-up: Changed 8 years ago by jelte

  • Owner changed from jelte to vorner

one style issue; the nameCompatible() method name isn't following the
style guideline (should be share_compatible())

In general the code looks ok, the spaghetti of sets and dicts is a bit
worrying though. Not so much as that it is too complicated as it is,
but what worries me is when other people might need to change
something in the future. But apart from adding some graph somewhere
which will no doubt go stale I don't really have any suggestion for it,
so you may just ignore this worry :p

And some minor, debatable style issues, both of there are a matter of
taste though;

there are a few instances where emptiness is checked with 'foo ==
set()'. I'd personally prefer len(foo) == 0

similarly, sets have a copy() method, which i would personally
choose over set(foo)

anyhow, these are minor, and apart from the name thing in the
beginning, I think this code can be merged.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 8 years ago by vorner

  • Owner changed from vorner to jelte

Hello

Replying to jelte:

one style issue; the nameCompatible() method name isn't following the
style guideline (should be share_compatible())

Ah, of course. I'm still mixing python and C++ styles.

In general the code looks ok, the spaghetti of sets and dicts is a bit
worrying though. Not so much as that it is too complicated as it is,
but what worries me is when other people might need to change
something in the future. But apart from adding some graph somewhere
which will no doubt go stale I don't really have any suggestion for it,
so you may just ignore this worry :p

Well, there would be other option, which is slower and probably more work to code, but it would be simpler with regards to the structure. The sockets could be in a list and a linear search for whatever is needed would be performed each time. The advantage would be there would be only one list, not multiple indexing hashes and stuff. But I personally prefer this approach, as it is cleaner design-wise and is already written. Or should I change it?

there are a few instances where emptiness is checked with 'foo ==
set()'. I'd personally prefer len(foo) == 0

Hmm, changed. But not in the tests, as assertEqual(set(), var) will produce nicer are more useful output when it goes wrong.

similarly, sets have a copy() method, which i would personally
choose over set(foo)

ACK.

anyhow, these are minor, and apart from the name thing in the
beginning, I think this code can be merged.

I'll merge it once the other two are reviewed. No need to have it in master yet.

With regards

comment:6 in reply to: ↑ 5 Changed 8 years ago by jelte

  • Owner changed from jelte to vorner

Replying to vorner:

Well, there would be other option, which is slower and probably more work to code, but it would be simpler with regards to the structure. The sockets could be in a list and a linear search for whatever is needed would be performed each time. The advantage would be there would be only one list, not multiple indexing hashes and stuff. But I personally prefer this approach, as it is cleaner design-wise and is already written. Or should I change it?

No this is fine

there are a few instances where emptiness is checked with 'foo ==
set()'. I'd personally prefer len(foo) == 0

Hmm, changed. But not in the tests, as assertEqual(set(), var) will produce nicer are more useful output when it goes wrong.

ack

I'll merge it once the other two are reviewed. No need to have it in master yet.

Ok

comment:7 Changed 8 years ago by vorner

  • Resolution set to complete
  • Status changed from reviewing to closed

Merged, closing.

Note: See TracTickets for help on using tickets.