backend/server/rhnServer/server_packages.py | 3 --- 1 file changed, 3 deletions(-)
New commits: commit 58f5b315e06d1057f306f7f258a1f3a3d3d1baf7 Author: Jan Pazdziora jpazdziora@redhat.com Date: Tue Sep 29 11:48:14 2009 +0200
No commit in processPackageKeyAssociations.
This change is based on the following communication:
Date: Wed, 16 Sep 2009 13:23:43 +0200 Subject: Is there a reason to have commit in processPackageKeyAssociations?
Hello,
looking at the source of processPackageKeyAssociations in server/rhnServer/server_packages.py, I can see rhnSQL.commit() there. Other functions in that file, even if they do inserts, do not commit -- I assume a higher level calls take care of committing.
Why do we commit in processPackageKeyAssociations?
It seems to break the expectation that the most top-level function decides whether to commit or rollback. Actually, we've just spent about two hours hunting through a testing Satellite, trying to figure out why something which has commit only at the end of the loop (updatePackages.py) behaves as if run in auto commit.
Pradeep, guys: do we have some documentation about what the recommended approach / coding style WRT transactions is?
-- Jan Pazdziora
---
The usual approach I would suggest is include all the dependent transactions into one commit, that way a rollback accounts for any anomolies. In processPackageKeyAssociations case its its own transaction as the package id is already created in the db and all we're doing is looking up the signature and associating it to a key and hence this method has its own commit as its an independent transaction. But if this is being called from updatePackages then the commit should happen at the end of the loop. I think we should be ok if we remove the rhnSQL.commit from processPackageKeyAssociations.
But in usual sense, always stack all the dependent transactions and commit or rollback at the end. Thats the recommended approach.
-- Pradeep Kilambi
---
So you think other pieces of code (satellite-sync, rhnpush handler) do their own commits anyway, so we should be fine?
-- Jan Pazdziora
---
Correct. Eventually, in the push_packages call we do commit at the end anyway. Which is after this call. Also for satsync we do commit in ChannelPackageSubscriptions which is again after this call. So I think we should be jus fine if we nuke this commit.
~ Prad
diff --git a/backend/server/rhnServer/server_packages.py b/backend/server/rhnServer/server_packages.py index 63ffd3e..b5a5156 100644 --- a/backend/server/rhnServer/server_packages.py +++ b/backend/server/rhnServer/server_packages.py @@ -436,7 +436,6 @@ def processPackageKeyAssociations(header, md5sum): lookup_keytype_id.execute() key_type_id = lookup_keytype_id.fetchone_dict() insert_keyid_sql.execute(key_id = key_id, key_type_id = key_type_id['id']) - rhnSQL.commit() lookup_keyid_sql.execute(key_id = key_id) keyid = lookup_keyid_sql.fetchall_dict()
@@ -447,8 +446,6 @@ def processPackageKeyAssociations(header, md5sum): if not exists_check: provider_sql.execute(key_id=keyid[0]['id'], package_id=pkg_id[0]['id'])
- rhnSQL.commit() -
# Compares list1 and list2 (each list is a tuple (n, v, r, e) # returns two lists
spacewalk-commits@lists.fedorahosted.org