Conditions | 55 |
Total Lines | 203 |
Code Lines | 156 |
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 |
||
140 | def post(self): |
||
141 | for inherit in self.scope.pop('inherit', []): |
||
142 | data = self.scope.template('neighbor', inherit) |
||
143 | self.scope.inherit(data) |
||
144 | local = self.scope.get() |
||
145 | |||
146 | neighbor = Neighbor() |
||
147 | |||
148 | # XXX: use the right class for the data type |
||
149 | # XXX: we can use the scope.nlri interface ( and rename it ) to set some values |
||
150 | neighbor.router_id = local.get('router-id', None) |
||
151 | neighbor.peer_address = local.get('peer-address', None) |
||
152 | neighbor.local_address = local.get('local-address', None) |
||
153 | neighbor.local_as = local.get('local-as', None) |
||
154 | neighbor.peer_as = local.get('peer-as', None) |
||
155 | neighbor.passive = local.get('passive', None) |
||
156 | neighbor.listen = local.get('listen', 0) |
||
157 | neighbor.connect = local.get('connect', 0) |
||
158 | neighbor.hold_time = local.get('hold-time', HoldTime(180)) |
||
159 | neighbor.rate_limit = local.get('rate-limit', 0) |
||
160 | neighbor.host_name = local.get('host-name', host()) |
||
161 | neighbor.domain_name = local.get('domain-name', domain()) |
||
162 | neighbor.md5_password = local.get('md5-password', None) |
||
163 | neighbor.md5_base64 = local.get('md5-base64', None) |
||
164 | neighbor.md5_ip = local.get('md5-ip', neighbor.local_address) |
||
165 | neighbor.description = local.get('description', '') |
||
166 | neighbor.flush = local.get('auto-flush', True) |
||
167 | neighbor.adj_rib_out = local.get('adj-rib-out', True) |
||
168 | neighbor.adj_rib_in = local.get('adj-rib-in', True) |
||
169 | neighbor.aigp = local.get('aigp', None) |
||
170 | neighbor.ttl_out = local.get('outgoing-ttl', None) |
||
171 | neighbor.ttl_in = local.get('incoming-ttl', None) |
||
172 | neighbor.group_updates = local.get('group-updates', True) |
||
173 | neighbor.manual_eor = local.get('manual-eor', False) |
||
174 | |||
175 | if neighbor.local_address is None: |
||
176 | return self.error.set('incomplete neighbor, missing local-address') |
||
177 | if neighbor.local_as is None: |
||
178 | return self.error.set('incomplete neighbor, missing local-as') |
||
179 | if neighbor.peer_as is None: |
||
180 | return self.error.set('incomplete neighbor, missing peer-as') |
||
181 | |||
182 | if neighbor.passive is None: |
||
183 | neighbor.passive = False |
||
184 | |||
185 | capability = local.get('capability', {}) |
||
186 | neighbor.nexthop = capability.get('nexthop', None) |
||
187 | neighbor.add_path = capability.get('add-path', 0) |
||
188 | neighbor.asn4 = capability.get('asn4', True) |
||
189 | neighbor.extended_message = capability.get('extended-message', True) |
||
190 | neighbor.multisession = capability.get('multi-session', False) |
||
191 | neighbor.operational = capability.get('operational', False) |
||
192 | neighbor.route_refresh = capability.get('route-refresh', 0) |
||
193 | |||
194 | if capability.get('graceful-restart', False) is not False: |
||
195 | neighbor.graceful_restart = capability.get('graceful-restart', 0) or int(neighbor.hold_time) |
||
196 | |||
197 | neighbor.api = ParseAPI.flatten(local.pop('api', {})) |
||
198 | |||
199 | families = [] |
||
200 | for family in ParseFamily.convert: |
||
201 | for pair in local.get('family', {}).get(family, []): |
||
202 | families.append(pair) |
||
203 | |||
204 | families = families or NLRI.known_families() |
||
205 | |||
206 | for family in families: |
||
207 | neighbor.add_family(family) |
||
208 | |||
209 | if neighbor.add_path: |
||
210 | add_path = local.get('add-path', {}) |
||
211 | if add_path: |
||
212 | for family in ParseAddPath.convert: |
||
213 | for pair in add_path.get(family, []): |
||
214 | if pair not in families: |
||
215 | log.debug( |
||
216 | 'skipping add-path family ' + str(pair) + ' as it is not negotiated', 'configuration' |
||
217 | ) |
||
218 | continue |
||
219 | neighbor.add_addpath(pair) |
||
220 | else: |
||
221 | for family in families: |
||
222 | neighbor.add_addpath(family) |
||
223 | |||
224 | # The default is to auto-detect by the presence of the nexthop block |
||
225 | # if this is manually set, then we honor it |
||
226 | nexthop = local.get('nexthop', {}) |
||
227 | if neighbor.nexthop is None and nexthop: |
||
228 | neighbor.nexthop = True |
||
229 | |||
230 | if neighbor.nexthop: |
||
231 | nexthops = [] |
||
232 | for family in nexthop: |
||
233 | nexthops.extend(nexthop[family]) |
||
234 | if nexthops: |
||
235 | for afi, safi, nhafi in nexthops: |
||
236 | if (afi, safi) not in neighbor.families(): |
||
237 | log.debug( |
||
238 | 'skipping nexthop afi,safi ' + str(afi) + '/' + str(safi) + ' as it is not negotiated', |
||
239 | 'configuration', |
||
240 | ) |
||
241 | continue |
||
242 | if (nhafi, safi) not in neighbor.families(): |
||
243 | log.debug( |
||
244 | 'skipping nexthop afi ' + str(nhafi) + '/' + str(safi) + ' as it is not negotiated', |
||
245 | 'configuration', |
||
246 | ) |
||
247 | continue |
||
248 | neighbor.add_nexthop(afi, safi, nhafi) |
||
249 | |||
250 | neighbor.changes = [] |
||
251 | neighbor.changes.extend(self.scope.pop_routes()) |
||
252 | |||
253 | # old format |
||
254 | for section in ('static', 'l2vpn', 'flow'): |
||
255 | routes = local.get(section, {}).get('routes', []) |
||
256 | for route in routes: |
||
257 | route.nlri.action = OUT.ANNOUNCE |
||
258 | neighbor.changes.extend(routes) |
||
259 | |||
260 | routes = local.get('routes', []) |
||
261 | for route in routes: |
||
262 | route.nlri.action = OUT.ANNOUNCE |
||
263 | neighbor.changes.extend(routes) |
||
264 | |||
265 | messages = local.get('operational', {}).get('routes', []) |
||
266 | |||
267 | if neighbor.local_address is None: |
||
268 | neighbor.auto_discovery = True |
||
269 | neighbor.local_address = None |
||
270 | neighbor.md5_ip = None |
||
271 | |||
272 | if not neighbor.router_id: |
||
273 | if neighbor.peer_address.afi == AFI.ipv4 and not neighbor.auto_discovery: |
||
274 | neighbor.router_id = neighbor.local_address |
||
275 | else: |
||
276 | return self.error.set('missing router-id for the peer, it can not be set using the local-ip') |
||
277 | |||
278 | if neighbor.route_refresh: |
||
279 | if neighbor.adj_rib_out: |
||
280 | log.debug('route-refresh requested, enabling adj-rib-out', 'configuration') |
||
281 | |||
282 | missing = neighbor.missing() |
||
283 | if missing: |
||
284 | return self.error.set('incomplete neighbor, missing %s' % missing) |
||
285 | |||
286 | if not neighbor.auto_discovery and neighbor.local_address.afi != neighbor.peer_address.afi: |
||
287 | return self.error.set('local-address and peer-address must be of the same family') |
||
288 | neighbor.range_size = neighbor.peer_address.mask.size() |
||
289 | |||
290 | if neighbor.range_size > 1 and not neighbor.passive: |
||
291 | return self.error.set('can only use ip ranges for the peer address with passive neighbors') |
||
292 | |||
293 | if neighbor.index() in self._neighbors: |
||
294 | return self.error.set('duplicate peer definition %s' % neighbor.peer_address.top()) |
||
295 | self._neighbors.append(neighbor.index()) |
||
296 | |||
297 | if neighbor.md5_password: |
||
298 | try: |
||
299 | md5 = base64.b64decode(neighbor.md5_password) if neighbor.md5_base64 else neighbor.md5_password |
||
300 | except TypeError as e: |
||
301 | return self.error.set("Invalid base64 encoding of MD5 password.") |
||
302 | else: |
||
303 | if len(md5) > 80: |
||
304 | return self.error.set('MD5 password must be no larger than 80 characters') |
||
305 | |||
306 | # check we are not trying to announce routes without the right MP announcement |
||
307 | for change in neighbor.changes: |
||
308 | family = change.nlri.family() |
||
309 | if family not in families and family != (AFI.ipv4, SAFI.unicast): |
||
310 | return self.error.set( |
||
311 | 'Trying to announce a route of type %s,%s when we are not announcing the family to our peer' |
||
312 | % change.nlri.family() |
||
313 | ) |
||
314 | |||
315 | def _init_neighbor(neighbor): |
||
316 | families = neighbor.families() |
||
317 | for change in neighbor.changes: |
||
318 | if change.nlri.family() in families: |
||
319 | # This add the family to neighbor.families() |
||
320 | neighbor.rib.outgoing.add_to_rib_watchdog(change) |
||
321 | for message in messages: |
||
322 | if message.family() in families: |
||
323 | if message.name == 'ASM': |
||
324 | neighbor.asm[message.family()] = message |
||
325 | else: |
||
326 | neighbor.messages.append(message) |
||
327 | self.neighbors[neighbor.name()] = neighbor |
||
328 | |||
329 | # create one neighbor object per family for multisession |
||
330 | if neighbor.multisession and len(neighbor.families()) > 1: |
||
331 | for family in neighbor.families(): |
||
332 | # XXX: FIXME: Ok, it works but it takes LOTS of memory .. |
||
333 | m_neighbor = deepcopy(neighbor) |
||
334 | m_neighbor.make_rib() |
||
335 | m_neighbor.rib.outgoing.families = [family] |
||
336 | _init_neighbor(m_neighbor) |
||
337 | else: |
||
338 | neighbor.make_rib() |
||
339 | _init_neighbor(neighbor) |
||
340 | |||
341 | local.clear() |
||
342 | return True |
||
343 |