search

multinet-app / multinet-api

Apache License 2.0
2 stars 2 forks source link

Add extended network creation method #20

Closed jjnesbitt closed 3 years ago

jjnesbitt commented 3 years ago

Since networks require some extra data on creation (an arango edge definition), we should add a method on the Network model which does all of the work of this code snippet:

https://github.com/multinet-app/multinet-api/blob/f4c79711bf9d0ef47aee581cbeba329e04605b27/multinet/api/views/network.py#L125-L142

I'm thinking something like get_or_create_with_edge_definition or something. The point being, it will return a network if it exists, or create one (and an underlying arango graph with the specified edge definition). This in a sense will act as an override to Network.object.get_or_create, although it's technically isn't.

waxlamp commented 3 years ago

When would this method be called? If you're in a position to try to create a network from an edge definition, doesn't that mean you believe it doesn't already exist?

Or put another way: what steps would lead you to call this method with an edge table in hand, if that network already exists?

jjnesbitt commented 3 years ago

If you're in a position to try to create a network from an edge definition, doesn't that mean you believe it doesn't already exist?

You may or may not believe it already exists, it's tangential to the goal of this method, which is to replicate the behavior of Network.objects.get_or_create. Networks need some additional information to correctly create arango graphs, which this method asks for (see implementation at https://github.com/multinet-app/multinet-api/pull/22/commits/eabf62d49701e80d3239f764c10bf3211ee40b34). This method should always be used instead of Network.objects.get_or_create, as we don't want to create a Network without creating the corresponding arango graph. It would put our two databases out of sync.

Without this method, if you wanted to create a network outside of the rest endpoint for doing such, you'd have to basically replicate the code in that rest endpoint (shown above).

Hopefully this clears things up.

waxlamp commented 3 years ago

Makes sense. I'm still confused about why you'd ever try to get a network with this function, but I'll just watch and see. Carry on.

jjnesbitt commented 3 years ago

Makes sense. I'm still confused about why you'd ever try to get a network with this function, but I'll just watch and see. Carry on.

You wouldn't, it's just a fail-safe to prevent desynchronization across arango and django. So if you call this function and see that created == False, then you can handle this case however you see fit, plus you have information on the network that already exists.