acquia_agent_check_subscription() will not detect subscription status change if existing id/key pair is replaced with another one. New parameter added to force the function to send status update notification.

CommentFileSizeAuthor
#10 1009810-status-update-10.patch3.02 KBpwolanin
#8 status_update.3.patch1.69 KBdmitry_bezer
#6 force_status_update.2.patch3.65 KBdmitry_bezer
force_status_update.patch2.62 KBdmitry_bezer

Comments

pwolanin’s picture

pwolanin commented
Status: Needs review » Needs work

It's not clear to me why that new parameter is needed.

dmitry_bezer’s picture

dmitry_bezer commented

acquia_agent_check_subscription() checks subscription status and calls acquia_subscription_status hook if it detected that the status has changed. The $force_status_update parameter is just intended to force the hook to be called if caller wants that.

pwolanin’s picture

pwolanin commented

Seems like the hook definition is changing then - previously you described it as being only invoked when the status changes.

What problem are you trying to solve here?

dmitry_bezer’s picture

dmitry_bezer commented

acquia_agent_check_subscription() via the hook notifies other modules if something was changed. The problem that it may not detect it in some cases, so caller of acquia_agent_check_subscription() just forces it to call the hook...

pwolanin’s picture

pwolanin commented

In that case, perhaps the design should change to just always send a notification, rather than trying to send just on change?

dmitry_bezer’s picture

dmitry_bezer commented
StatusFileSize
new force_status_update.2.patch3.65 KB

acquia_agent_check_subscription() is also periodically called by cron to send heartbeats to AN so the notification would be sent each time cron is run but the status is not actually changed.

attached is updated patch that also removes old code

pwolanin’s picture

pwolanin commented

What's the downside of making the call each time?

It still doesn't make sense to me to "force" the API call. It should either happen on change, or always.

dmitry_bezer’s picture

dmitry_bezer commented
StatusFileSize
new status_update.3.patch1.69 KB

Ok. Let the notification happen each time then.

dmitry_bezer’s picture

dmitry_bezer commented
Status: Needs work » Needs review
pwolanin’s picture

pwolanin commented
Version: 7.x-1.0-alpha1 » 7.x-1.0-beta1
StatusFileSize
new 1009810-status-update-10.patch3.02 KB

Seem like we should change the code in pages.inc too?

dmitry_bezer’s picture

dmitry_bezer commented

Yes. I missed that
thank you

pwolanin’s picture

pwolanin commented
Status: Needs review » Fixed

committed

Status: Fixed » Closed (fixed)

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