perform credentials check before creating consumer #256

abaruni · 2018-03-05T18:43:39Z

No description provided.

abaruni · 2018-03-06T01:24:23Z

there are two changes here:

Perform a check of the messagehub credentials (if it is messagehub) and topic. This is to ensure that the authkey and topic provide when the trigger was originally created are still valid. there is the possibility that credentials and/or a topic could be deleted without the trigger being deleted. this leaves around zombie consumers who never stop attempting to connect to a broker. the idea here is to do the check when creating a consumer for an active trigger. if the authkey and topic are valid, then we will create the consumer as usual. if either are invalid. then the consumer will not be created and the trigger will be disabled in the db. if encounter any errors while trying to validate, then we'll retry. if at the end of the alloted retries, we don't succeed in getting either a 200 or 403, then we'll treat it as though we did get a 200. the motivation being that we'll eventually catch it on subsequent deploys. better to err on the side of caution than to mistakenly disable a valid trigger
do not create consumers for disabled triggers

dubee · 2018-03-06T04:02:07Z

provider/service.py

@@ -106,7 +113,7 @@ def run(self):
                                    # running trigger should become disabled
                                    # this should be done regardless of which worker the document claims to be assigned to
                                    logging.info('[{}] Existing running trigger should become disabled'.format(change["id"]))
-                                    existingConsumer.disable()
+                                    existingConsumer.shutdown()


Need to update doc above that mentions that the consumer should be disabled.

dubee · 2018-03-06T04:02:49Z

provider/service.py

+        if doc['isMessageHub']:
+            retries = 0
+            max_retries = 5
+            url = '{}/admin/topics'.format(doc['kafka_admin_url'])


Will the admin URL throttle requests?

haven't experienced throttling in my own testing, but i will have to double check on the official answer

dubee · 2018-03-06T04:09:10Z

provider/service.py

-            consumer.start()
-        else:
-            logging.info('[{}] Trigger was determined to be disabled, not starting...'.format(triggerFQN))
+        logging.info('[{}] Starting consumer...'.format(triggerFQN))


Do these changes affect the other occurrence of the call to __isTriggerDocActive in this file?

yes. currently we detect a change in the changes feed and check whether that consumer is in our in-memory cache. this is still the case, however, since we will not be creating disabled consumers. the only time we'll hit the code path where we are going from inactive -> active and

we do have that consumer in memory is when we disable a trigger for reasons like client errors. doing that is something that may likely change in the future. so that whole logic would need reworking

dubee · 2018-03-06T04:10:25Z

provider/service.py

+            password = doc['password']
+            topic = doc['topic']
+
+            while retries < max_retries:


Seems like we could have a test here for invalid auth.

yeah i'm afraid i'm going to have to change a doc in cloudant for that. need to get that figured out

abaruni · 2018-03-06T04:24:37Z

provider/service.py

+                    logging.info('Received status code {} for {} while validating authKey. Retrying in {} second(s)'.format(status_code, triggerFQN, sleepyTime))
+                    time.sleep(sleepyTime)
+                    retries += 1
+
        # Create a representation for this trigger, even if it is disabled


i need to update this comment as well

… and shutting down vs disabling

dgrove-oss · 2020-05-26T13:19:22Z

Closing as abandoned

perform credentials check before creating consumer

4b41f4b

disable trigger when topic does not exit

bdd560b

dubee requested changes Mar 6, 2018

View reviewed changes

dubee reviewed Mar 6, 2018

View reviewed changes

abaruni commented Mar 6, 2018

View reviewed changes

update code comments to reflect no longer creating disabled consumers…

954d9a0

… and shutting down vs disabling

dgrove-oss closed this May 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perform credentials check before creating consumer #256

perform credentials check before creating consumer #256

abaruni commented Mar 5, 2018

abaruni commented Mar 6, 2018

dubee Mar 6, 2018

abaruni Mar 6, 2018

dubee Mar 6, 2018

abaruni Mar 6, 2018

dubee Mar 6, 2018

abaruni Mar 6, 2018

abaruni Mar 6, 2018

dubee Mar 6, 2018

abaruni Mar 6, 2018

abaruni Mar 6, 2018

dgrove-oss commented May 26, 2020

perform credentials check before creating consumer #256

perform credentials check before creating consumer #256

Conversation

abaruni commented Mar 5, 2018

abaruni commented Mar 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgrove-oss commented May 26, 2020