Optimize the category PK search
Closed this issue · 7 comments
Hey,
I have recently added the name field of the Categories to the filterable columns in InvenTree:
inventree/InvenTree#3854
This would allow to make this code to be made more efficient:
with simplifying to:
# Fetch all categories
part_categories = PartCategory.list(inventree_api, name=category_name)
if len(part_categories) == 1:
return part_categories[0].pk
return -1
This change got introduced with the InvenTree API version 78, the inventree-python depends on the version 51:
https://github.com/inventree/inventree-python/blob/master/inventree/api.py#L25
It might be possible to add a condition based on the server's API version?
@martonmiklos Where is the gain in your experience and by how much? Adding conditions based on API version is going to be tough to maintain... it would be preferable to request the InvenTree Python API to catch up and up the Ki-nTree dependency. What do you think?
Where is the gain in your experience and by how much?
Well I imported my category tree from the TME and it consists ~1700 categories.
What do you think?
Increasing the min API version in the python API would mean that the python API would drop support for older InvenTree versions which I think until a point when it is not necessary should be avoidable.
I do not know how wide is the Ki-n-tree user base, but the best possible option would be setting an API barrier here, and throw error if lower API version of InvenTree is connected.
The thing is, I'm not really sure where the gain is... I don't have any performance issue using Ki-nTree.
So I don't find value in adding an unnecessary constraint, unless it achieves really great rewards.
Just to take some numbers to the desk I put together the following script:
https://gist.github.com/martonmiklos/20df2a75de77cee06d6dc755e65bdb0e
It gives me the following run time (with 1618 categories):
(env) mm@lapos:/tmp$ python3 benchmark.py
test category name match
Name match only: 330.321550 ms
test category name match
Name and parent pk match: 351.499796 ms
Filter using InvenTree: 33.458471 ms
I have not traced the Kin-Tree yet how often does this category filter function get called, but at some point I would like to add mass import feature from CSV to it where the 30 vs. 300 ms ish time could count something.
My recommendation would be to keep this in mind and at some point (maybe when the InvenTree python API sets the min. level above the necessary) we can drop the current code.
Ok that's a nice improvement and yes I agree with the strategy of waiting till older versions of InvenTree become really old to assume we can make the change.
It is related to the implementation you did in #87 right?
It is related to the implementation you did in #87 right?
Well I have been worked on an Arena PLM migration tool where implementing it became necessary to implement it in InvenTree.
I just peeked the code during the #87 and I came across it came into my mind.
But it does not necessary to implement my plans in #87.
Thanks @martonmiklos. Currently on InvenTree's 0.8.x
branch the latest API version is 69
, quite a bit behind! (no pun intended)
I would guess that for the imminent 0.9.0
release the API version will jump to 80
or later. If that's the case, I will update the get_inventree_category_id
method with your proposed change and note it in the release notes so people are aware it could be a breaking change if they don't upgrade InvenTree to latest version.
Sounds reasonable?