Conditions | 55 |
Total Lines | 192 |
Code Lines | 149 |
Lines | 0 |
Ratio | 0 % |
Changes | 0 |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
Complex classes like exabgp.configuration.neighbor.ParseNeighbor.post() often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
1 | # encoding: utf-8 |
||
136 | def post (self): |
||
137 | for inherit in self.scope.pop('inherit',[]): |
||
138 | data = self.scope.template('neighbor',inherit) |
||
139 | self.scope.inherit(data) |
||
140 | local = self.scope.get() |
||
141 | |||
142 | neighbor = Neighbor() |
||
143 | |||
144 | # XXX: use the right class for the data type |
||
145 | # XXX: we can use the scope.nlri interface ( and rename it ) to set some values |
||
146 | neighbor.router_id = local.get('router-id',None) |
||
147 | neighbor.peer_address = local.get('peer-address',None) |
||
148 | neighbor.local_address = local.get('local-address',None) |
||
149 | neighbor.local_as = local.get('local-as',None) |
||
150 | neighbor.peer_as = local.get('peer-as',None) |
||
151 | neighbor.passive = local.get('passive',None) |
||
152 | neighbor.listen = local.get('listen',0) |
||
153 | neighbor.connect = local.get('connect',0) |
||
154 | neighbor.hold_time = local.get('hold-time',HoldTime(180)) |
||
155 | neighbor.rate_limit = local.get('rate-limit',0) |
||
156 | neighbor.host_name = local.get('host-name',host()) |
||
157 | neighbor.domain_name = local.get('domain-name',domain()) |
||
158 | neighbor.md5_password = local.get('md5-password',None) |
||
159 | neighbor.md5_base64 = local.get('md5-base64', None) |
||
160 | neighbor.md5_ip = local.get('md5-ip',neighbor.local_address) |
||
161 | neighbor.description = local.get('description','') |
||
162 | neighbor.flush = local.get('auto-flush',True) |
||
163 | neighbor.adj_rib_out = local.get('adj-rib-out',True) |
||
164 | neighbor.adj_rib_in = local.get('adj-rib-in',True) |
||
165 | neighbor.aigp = local.get('aigp',None) |
||
166 | neighbor.ttl_out = local.get('outgoing-ttl',None) |
||
167 | neighbor.ttl_in = local.get('incoming-ttl',None) |
||
168 | neighbor.group_updates = local.get('group-updates',True) |
||
169 | neighbor.manual_eor = local.get('manual-eor', False) |
||
170 | |||
171 | if neighbor.local_address is None: |
||
172 | return self.error.set('incomplete neighbor, missing local-address') |
||
173 | if neighbor.local_as is None: |
||
174 | return self.error.set('incomplete neighbor, missing local-as') |
||
175 | if neighbor.peer_as is None: |
||
176 | return self.error.set('incomplete neighbor, missing peer-as') |
||
177 | |||
178 | if neighbor.passive is None: |
||
179 | neighbor.passive = False |
||
180 | |||
181 | capability = local.get('capability',{}) |
||
182 | neighbor.nexthop = capability.get('nexthop',None) |
||
183 | neighbor.add_path = capability.get('add-path',0) |
||
184 | neighbor.asn4 = capability.get('asn4',True) |
||
185 | neighbor.extended_message = capability.get('extended-message',True) |
||
186 | neighbor.multisession = capability.get('multi-session',False) |
||
187 | neighbor.operational = capability.get('operational',False) |
||
188 | neighbor.route_refresh = capability.get('route-refresh',0) |
||
189 | |||
190 | if capability.get('graceful-restart',False) is not False: |
||
191 | neighbor.graceful_restart = capability.get('graceful-restart',0) or int(neighbor.hold_time) |
||
192 | |||
193 | neighbor.api = ParseAPI.flatten(local.pop('api',{})) |
||
194 | |||
195 | families = [] |
||
196 | for family in ParseFamily.convert: |
||
197 | for pair in local.get('family',{}).get(family,[]): |
||
198 | families.append(pair) |
||
199 | |||
200 | families = families or NLRI.known_families() |
||
201 | |||
202 | for family in families: |
||
203 | neighbor.add_family(family) |
||
204 | |||
205 | if neighbor.add_path: |
||
206 | add_path = local.get('add-path',{}) |
||
207 | if add_path: |
||
208 | for family in ParseAddPath.convert: |
||
209 | for pair in add_path.get(family,[]): |
||
210 | if pair not in families: |
||
211 | self.logger.debug('skipping add-path family ' + str(pair) + ' as it is not negotiated','configuration') |
||
212 | continue |
||
213 | neighbor.add_addpath(pair) |
||
214 | else: |
||
215 | for family in families: |
||
216 | neighbor.add_addpath(family) |
||
217 | |||
218 | # The default is to auto-detect by the presence of the nexthop block |
||
219 | # if this is manually set, then we honor it |
||
220 | nexthop = local.get('nexthop', {}) |
||
221 | if neighbor.nexthop is None and nexthop: |
||
222 | neighbor.nexthop = True |
||
223 | |||
224 | if neighbor.nexthop: |
||
225 | nexthops = [] |
||
226 | for family in nexthop: |
||
227 | nexthops.extend(nexthop[family]) |
||
228 | if nexthops: |
||
229 | for afi,safi,nhafi in nexthops: |
||
230 | if (afi,safi) not in neighbor.families(): |
||
231 | self.logger.debug('skipping nexthop afi,safi ' + str(afi) + '/' + str(safi) + ' as it is not negotiated', 'configuration') |
||
232 | continue |
||
233 | if (nhafi, safi) not in neighbor.families(): |
||
234 | self.logger.debug('skipping nexthop afi ' + str(nhafi) + '/' + str(safi) + ' as it is not negotiated', 'configuration') |
||
235 | continue |
||
236 | neighbor.add_nexthop(afi, safi, nhafi) |
||
237 | |||
238 | neighbor.changes = [] |
||
239 | neighbor.changes.extend(self.scope.pop_routes()) |
||
240 | |||
241 | # old format |
||
242 | for section in ('static','l2vpn','flow'): |
||
243 | routes = local.get(section,{}).get('routes',[]) |
||
244 | for route in routes: |
||
245 | route.nlri.action = OUT.ANNOUNCE |
||
246 | neighbor.changes.extend(routes) |
||
247 | |||
248 | routes = local.get('routes',[]) |
||
249 | for route in routes: |
||
250 | route.nlri.action = OUT.ANNOUNCE |
||
251 | neighbor.changes.extend(routes) |
||
252 | |||
253 | messages = local.get('operational',{}).get('routes',[]) |
||
254 | |||
255 | if neighbor.local_address is None: |
||
256 | neighbor.auto_discovery = True |
||
257 | neighbor.local_address = None |
||
258 | neighbor.md5_ip = None |
||
259 | |||
260 | if not neighbor.router_id: |
||
261 | if neighbor.peer_address.afi == AFI.ipv4 and not neighbor.auto_discovery: |
||
262 | neighbor.router_id = neighbor.local_address |
||
263 | else: |
||
264 | return self.error.set('missing router-id for the peer, it can not be set using the local-ip') |
||
265 | |||
266 | if neighbor.route_refresh: |
||
267 | if neighbor.adj_rib_out: |
||
268 | self.logger.debug('route-refresh requested, enabling adj-rib-out','configuration') |
||
269 | |||
270 | missing = neighbor.missing() |
||
271 | if missing: |
||
272 | return self.error.set('incomplete neighbor, missing %s' % missing) |
||
273 | |||
274 | if not neighbor.auto_discovery and neighbor.local_address.afi != neighbor.peer_address.afi: |
||
275 | return self.error.set('local-address and peer-address must be of the same family') |
||
276 | neighbor.range_size = neighbor.peer_address.mask.size() |
||
277 | |||
278 | if neighbor.range_size > 1 and not neighbor.passive: |
||
279 | return self.error.set('can only use ip ranges for the peer address with passive neighbors') |
||
280 | |||
281 | if neighbor.index() in self._neighbors: |
||
282 | return self.error.set('duplicate peer definition %s' % neighbor.peer_address.top()) |
||
283 | self._neighbors.append(neighbor.index()) |
||
284 | |||
285 | if neighbor.md5_password: |
||
286 | try: |
||
287 | md5 = base64.b64decode(neighbor.md5_password) if neighbor.md5_base64 else neighbor.md5_password |
||
288 | except TypeError as e: |
||
289 | return self.error.set("Invalid base64 encoding of MD5 password.") |
||
290 | else: |
||
291 | if len(md5) > 80: |
||
292 | return self.error.set('MD5 password must be no larger than 80 characters') |
||
293 | |||
294 | # check we are not trying to announce routes without the right MP announcement |
||
295 | for change in neighbor.changes: |
||
296 | family = change.nlri.family() |
||
297 | if family not in families and family != (AFI.ipv4,SAFI.unicast): |
||
298 | return self.error.set('Trying to announce a route of type %s,%s when we are not announcing the family to our peer' % change.nlri.family()) |
||
299 | |||
300 | def _init_neighbor (neighbor): |
||
301 | families = neighbor.families() |
||
302 | for change in neighbor.changes: |
||
303 | if change.nlri.family() in families: |
||
304 | # This add the family to neighbor.families() |
||
305 | neighbor.rib.outgoing.add_to_rib_watchdog(change) |
||
306 | for message in messages: |
||
307 | if message.family() in families: |
||
308 | if message.name == 'ASM': |
||
309 | neighbor.asm[message.family()] = message |
||
310 | else: |
||
311 | neighbor.messages.append(message) |
||
312 | self.neighbors[neighbor.name()] = neighbor |
||
313 | |||
314 | # create one neighbor object per family for multisession |
||
315 | if neighbor.multisession and len(neighbor.families()) > 1: |
||
316 | for family in neighbor.families(): |
||
317 | # XXX: FIXME: Ok, it works but it takes LOTS of memory .. |
||
318 | m_neighbor = deepcopy(neighbor) |
||
319 | m_neighbor.make_rib() |
||
320 | m_neighbor.rib.outgoing.families = [family] |
||
321 | _init_neighbor(m_neighbor) |
||
322 | else: |
||
323 | neighbor.make_rib() |
||
324 | _init_neighbor(neighbor) |
||
325 | |||
326 | local.clear() |
||
327 | return True |
||
328 |