Edge refactoring to DAL and minor PIP improvements #671
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #671 +/- ##
===========================================
+ Coverage 56.62% 58.68% +2.05%
===========================================
Files 120 127 +7
Lines 4044 4216 +172
===========================================
+ Hits 2290 2474 +184
+ Misses 1754 1742 -12
Continue to review full report at Codecov.
|
| displayed_edge["os"] = os | ||
| # we need to deepcopy all mutable edge properties, because weak-reference link is made otherwise, | ||
| # which is destroyed after method is exited and causes an error later. | ||
| displayed_edge["exploits"] = deepcopy(edge['exploits']) |
There was a problem hiding this comment.
Edge is a weak-reference link to mongoengine document. This link is destroyed once we exit the method where edge = Edge.objects.get(). I'm not sure how we can make this less error prone, maybe refactor exploits into embedded document...
|
|
||
| @staticmethod | ||
| def get_or_create_edge(src_node_id, dst_node_id, src_label, dst_label): | ||
| edge = False |
There was a problem hiding this comment.
I think it'll be better to start with None and in the finally use if edge is None
| edge.src_label = src_label | ||
| edge.dst_label = dst_label | ||
| edge.save() |
There was a problem hiding this comment.
Shouldn't we reuse update_label here?
| edge.tunnel = False | ||
| edge.save() | ||
| except DoesNotExist: | ||
| pass |
There was a problem hiding this comment.
Not exactly, but solved
| displayed_edge = DisplayedEdgeService.edge_to_displayed_edge(edge, for_report) | ||
| return displayed_edge | ||
|
|
||
| @staticmethod |
There was a problem hiding this comment.
Why is this a static method and not a regular method of Edge?
There was a problem hiding this comment.
I wanted to separate the data structure/schema of Edge document from methods, in order not to have a single huge file. But you're right - we can do it without static method :) And I should make it into a boundary.
| else: | ||
| return [x + ": " + (services[x]['name'] if 'name' in services[x] else 'unknown') for x in services] | ||
|
|
||
| @staticmethod |
There was a problem hiding this comment.
Why is this a static method and not a regular method of Edge?
What is this?
Fixes part of #662
Add any further explanations here.
Checklist
Changes
Refactored edges to use DAL
Minor PIP improvements