From 47fa4887d07a959e3cee0fd4801a5bde9d525769 Mon Sep 17 00:00:00 2001 From: Stephen Knox Date: Tue, 28 Apr 2026 12:33:01 +0100 Subject: [PATCH 1/2] Make changes to enable add_attributes to operate more qiuckly --- hydra_base/lib/attributes.py | 64 +++++++++++++++++++++-------- tests/attributes/test_attributes.py | 2 - 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/hydra_base/lib/attributes.py b/hydra_base/lib/attributes.py index 15ef1744..4fb36a0a 100644 --- a/hydra_base/lib/attributes.py +++ b/hydra_base/lib/attributes.py @@ -297,6 +297,10 @@ def _add_attribute(attr, user_id, flush=True, do_reassign=False): if flush is True: db.DBSession.flush() + # Return ORM object if not flushed (to get ID after batch flush) + # Return JSONObject if already flushed + if flush is False: + return attr_i return JSONObject(attr_i) def _reassign_scoped_attributes(attr_id, user_id): @@ -625,8 +629,29 @@ def add_attributes(attrs, **kwargs): #add a new attribute. user_id = kwargs.get('user_id') - # All global attributes - global_attrs = db.DBSession.query(Attr).filter(and_(Attr.network_id == None, Attr.project_id == None)).all() + # Extract incoming attr keys to filter queries and avoid loading unnecessary rows + incoming_attr_keys = [] + for a in attrs: + if a is not None: + incoming_attr_keys.append((a.name.lower(), a.dimension_id)) + + if not incoming_attr_keys: + return [] + + # Build OR conditions for filtering queries by (name, dimension_id) pairs + name_dim_filters = [ + and_(func.lower(Attr.name) == name, Attr.dimension_id == dim) + for name, dim in incoming_attr_keys + ] + + # All global attributes matching incoming names/dimensions + global_attrs = db.DBSession.query(Attr).filter( + and_( + Attr.network_id == None, + Attr.project_id == None, + or_(*name_dim_filters) + ) + ).all() #project-scoped attributes project_attrs = [] @@ -658,21 +683,22 @@ def add_attributes(attrs, **kwargs): #go top down, saving the attributes, and overwriting the duplicates as we #go down the tree. project_attr_dict = {} - seen = [] - for project_id in project_ids + network_project_ids: - # avoid duplicates. Can't use a set here because order is important - if project_id in seen: - continue - else: - seen.append(project_id) - project_attrs = db.DBSession.query(Attr).filter(Attr.project_id == project_id).all() + if project_ids + network_project_ids: + # Query for project attrs matching incoming keys in a single query + project_attrs = db.DBSession.query(Attr).filter( + Attr.project_id.in_(project_ids + network_project_ids), + or_(*name_dim_filters) + ).all() for project_attr in project_attrs: - project_attr_dict[(project_attr.name, project_attr.dimension)] = project_attr + project_attr_dict[(project_attr.name, project_attr.dimension_id)] = project_attr #network scoped attributes network_attrs = [] if len(network_ids) > 0: - network_attrs = db.DBSession.query(Attr).filter(Attr.network_id.in_(network_ids)).all() + network_attrs = db.DBSession.query(Attr).filter( + Attr.network_id.in_(network_ids), + or_(*name_dim_filters) + ).all() all_attrs = global_attrs + list(project_attr_dict.values()) + network_attrs @@ -694,16 +720,22 @@ def add_attributes(attrs, **kwargs): else: attrs_to_add.append(JSONObject(potential_new_attr)) - new_attrs = [] + # Batch insert: collect ORM objects without flushing individually + orm_attrs = [] for attr in attrs_to_add: - new_attr_i = _add_attribute(attr, flush=True, user_id=user_id) - new_attrs.append(new_attr_i) + new_attr_i = _add_attribute(attr, flush=False, user_id=user_id) + orm_attrs.append(new_attr_i) + # Single flush for all new attributes instead of per-attribute flushes db.DBSession.flush() + # Convert ORM objects to JSONObjects AFTER flush so they have IDs assigned + new_attrs = [JSONObject(a) for a in orm_attrs] + + # Combine with existing attributes (which are already JSONObjects) new_attrs = new_attrs + existing_attrs - return [JSONObject(a) for a in new_attrs] + return new_attrs def get_attributes(network_id=None, project_id=None, diff --git a/tests/attributes/test_attributes.py b/tests/attributes/test_attributes.py index 581fc219..a0fd755a 100644 --- a/tests/attributes/test_attributes.py +++ b/tests/attributes/test_attributes.py @@ -112,8 +112,6 @@ def test_update_attribute(self, client): with pytest.raises(HydraError): client.update_attribute(new_attr_fail) - - def test_delete_attribute(self, client): test_attr = JSONObject({ "name": 'Test Attribute 1', From f1f956cb69acb7daec741757e05b8dfad6453c59 Mon Sep 17 00:00:00 2001 From: Stephen Knox Date: Wed, 24 Jun 2026 09:38:00 +0000 Subject: [PATCH 2/2] Address the issues identified in the review: Make project / network insertions more deterministic. 2. bypass check_can_add_attribute for greater speed. and 3. use a lookup table to check for uniqueness instead of using a DB constraint --- hydra_base/lib/attributes.py | 56 ++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/hydra_base/lib/attributes.py b/hydra_base/lib/attributes.py index 4fb36a0a..7c8fd4cd 100644 --- a/hydra_base/lib/attributes.py +++ b/hydra_base/lib/attributes.py @@ -629,28 +629,18 @@ def add_attributes(attrs, **kwargs): #add a new attribute. user_id = kwargs.get('user_id') - # Extract incoming attr keys to filter queries and avoid loading unnecessary rows - incoming_attr_keys = [] - for a in attrs: - if a is not None: - incoming_attr_keys.append((a.name.lower(), a.dimension_id)) + # Deduplicate to unique lowercase names for an efficient IN filter. + # Exact (name, dimension_id) matching is done in Python via attr_dict below. + unique_lower_names = list({a.name.lower() for a in attrs if a is not None}) - if not incoming_attr_keys: + if not unique_lower_names: return [] - # Build OR conditions for filtering queries by (name, dimension_id) pairs - name_dim_filters = [ - and_(func.lower(Attr.name) == name, Attr.dimension_id == dim) - for name, dim in incoming_attr_keys - ] - - # All global attributes matching incoming names/dimensions + # All global attributes matching incoming names global_attrs = db.DBSession.query(Attr).filter( - and_( - Attr.network_id == None, - Attr.project_id == None, - or_(*name_dim_filters) - ) + Attr.network_id == None, + Attr.project_id == None, + func.lower(Attr.name).in_(unique_lower_names) ).all() #project-scoped attributes @@ -682,14 +672,17 @@ def add_attributes(attrs, **kwargs): #go top down, saving the attributes, and overwriting the duplicates as we #go down the tree. + all_project_ids = project_ids + network_project_ids project_attr_dict = {} - if project_ids + network_project_ids: - # Query for project attrs matching incoming keys in a single query + if all_project_ids: project_attrs = db.DBSession.query(Attr).filter( - Attr.project_id.in_(project_ids + network_project_ids), - or_(*name_dim_filters) + Attr.project_id.in_(all_project_ids), + func.lower(Attr.name).in_(unique_lower_names) ).all() - for project_attr in project_attrs: + # all_project_ids is ordered parent-first (root → leaf); sorting by that + # order ensures child entries overwrite parent entries for the same key. + project_id_order = {pid: i for i, pid in enumerate(all_project_ids)} + for project_attr in sorted(project_attrs, key=lambda a: project_id_order[a.project_id]): project_attr_dict[(project_attr.name, project_attr.dimension_id)] = project_attr #network scoped attributes @@ -697,7 +690,7 @@ def add_attributes(attrs, **kwargs): if len(network_ids) > 0: network_attrs = db.DBSession.query(Attr).filter( Attr.network_id.in_(network_ids), - or_(*name_dim_filters) + func.lower(Attr.name).in_(unique_lower_names) ).all() all_attrs = global_attrs + list(project_attr_dict.values()) + network_attrs @@ -720,9 +713,22 @@ def add_attributes(attrs, **kwargs): else: attrs_to_add.append(JSONObject(potential_new_attr)) + # Deduplicate to prevent IntegrityError when a caller submits two attrs + # with the same (name, dimension_id, scope) in one batch — the deferred + # flush means _check_can_add_attribute can't catch in-batch duplicates. + seen_new_keys = set() + deduped_attrs_to_add = [] + for attr in attrs_to_add: + dedup_key = (attr.name.lower(), attr.dimension_id, + getattr(attr, 'project_id', None), + getattr(attr, 'network_id', None)) + if dedup_key not in seen_new_keys: + seen_new_keys.add(dedup_key) + deduped_attrs_to_add.append(attr) + # Batch insert: collect ORM objects without flushing individually orm_attrs = [] - for attr in attrs_to_add: + for attr in deduped_attrs_to_add: new_attr_i = _add_attribute(attr, flush=False, user_id=user_id) orm_attrs.append(new_attr_i)