Renaming an internal client name tries to insert a new quota, and causes the following error:

user warning: Duplicate entry '338-sites' for key 'PRIMARY' query: INSERT INTO hosting_client_quota (client, resource, value) VALUES (338, 'sites', '0') in /var/aegir/hostmaster-6.x-1.2/profiles/hostmaster/modules/hosting/quota/hosting_quota.module on line 154.

Comments

ergonlogic’s picture

Title: Renaming an internal client name throws a warning about quotas » Updating client throws a warning about quotas
Assigned: Unassigned » ergonlogic
Status: Active » Needs review

Committed fix to my dev repo: http://drupalcode.org/sandbox/ergonlogic/1226310.git/commit/9688a7015129...

Also, this error was thrown whenever a client node was updated, but the quota not changed. So, I've changed the title of this issue to reflect that.

Steven Jones’s picture

Status: Needs review » Needs work

Hmm...I think that maybe we should just try to fix the hosting_quota_set_limit function so that we can call it safely at all times?

We're trying to do a merge query, and in D6 the best attempt at one is to do a SELECT and then decide on a INSERT or UPDATE.

ergonlogic’s picture

OK, I'll take a look at making hosting_quota_set_limit() safe.

I still think the absense of a hosting_quota_get_limit() function is an oversight. I guess it would be a change to the API, though...

ergonlogic’s picture

Status: Needs work » Needs review

Please take a look at http://drupalcode.org/sandbox/ergonlogic/1226310.git/commit/ddf76d426cba... for a safe implementation of hosting_quota_set_limit()

Steven Jones’s picture

Status: Needs review » Needs work

I'm happy with adding a hosting_quota_get_limit function, api additions are slightly different from api changes.

Detailed review:

The documentation of hosting_quota_set_limit needs some work, most lines need a full-stop at the end, and we don't indicate the type of parameters, so you can drop 'int', and 'string' etc.

function hosting_quota_set_limit ($client, $resource, $value) {

There's an extra space between the function name and the arguments.

Can you rewrite the hosting_quota_set_limit to use the merge query 'degenerate' pattern, this is described here for D7: http://drupal.org/node/310085. Essentially, if we just do a SELECT count to see if we need to do an UPDATE or an INSERT then we can just do it. I wouldn't expect it to call hosting_quota_get_limitbtw.

Once all that's done, we should be good to go!

ergonlogic’s picture

OK, this should implement the "degenerate" pattern: http://drupalcode.org/sandbox/ergonlogic/1226310.git/commit/6718a154eb18...

Let me know if that'll work.

Sorry for the delay in posting this.

Steven Jones’s picture

Status: Needs work » Fixed

Thanks for the fix!

It was almost there, just missing a db_result, but I added that in for you in:
http://drupalcode.org/project/hostmaster.git/commit/e719fab273b9c9f7608a...

Tested and pushed to both branches. Many thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit 419e4f7 on 7.x-2.x, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-588728-views-integration, dev-1403208-new_roles, dev-helmo-3.x by Steven Jones:
    Issue #1238560 - Fixes Updating client throws a warning about quotas.
    
    

  • Commit 419e4f7 on 7.x-2.x, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-588728-views-integration, dev-1403208-new_roles, dev-helmo-3.x by Steven Jones:
    Issue #1238560 - Fixes Updating client throws a warning about quotas.