Fixed issue 20 and 22 #23
Conversation
…at, added retry logic for ec2 api calls
| # these 7 lines are handling for legacy DDB items created prior how items were written | ||
| instance_attribute = instance_attribute.replace("'", '"') | ||
| instance_attribute = instance_attribute.replace(" True,", ' "True",') | ||
| instance_attribute = instance_attribute.replace(" True}", ' "True"}') | ||
| instance_attribute = instance_attribute.replace(" True,", ' "True",') | ||
| instance_attribute = instance_attribute.replace(" False,", ' "False",') | ||
| instance_attribute = instance_attribute.replace(" False,", ' "False",') | ||
| instance_attribute = instance_attribute.replace(" False,", ' "False",') |
There was a problem hiding this comment.
Why this instead of the DDB serialization/deserialization calls?
There was a problem hiding this comment.
those are only to cover DDB items created prior to this code change. The DDB serial/deserial code doesn't work for us because the DDB items is just 1 attribute (InstanceAttributes). All of the Instance Attribute is combined into a single string field and written in to the InstanceAttributes, so the code to serialize/deserialize doesn't add anything.
So simple json.dumps and json.loads of the string before and after does the trick to handle the various double quotes, true, True, false, False differences between.
Anyway, those 7 lines above, I kept because there are several (27 currently) DDB items that were written in the "old" method. So to correctly parse them, those 7 lines will/should handle those 27 legacy items.
Any new DDB items will not require those 7 lines (and replace will find nothing to replace) since the string written in the InstanceAttributes field should already be in the correct string format (double quotes and lower case true/false/none). So when performing json.loads, it will handle it better.
There was a problem hiding this comment.
If we deleted ALL existing 27 items (meaning the associated A/PTR/TXT records were deleted) then we could have taken out the 7 commands above.
There was a problem hiding this comment.
Thanks for the explanation. This should not be needed in any new implementations, and in theory if we scrub ma8-gov and start fresh, we'd not need them either, right?
There was a problem hiding this comment.
Right - if we somehow were able to scrub the 27 existing records, Then yes, the 7 lines above that I have put in, can be "removed". I'm not sure what's the easiest way to scrub these:
All of these of course requires that we update the Lambda with the latest.
- Stop and restart the associated ec2 instance(s). This will delete the DDB item and re-create it with the proper formatted.
- Manually retrieve the DDB item and reformat the
InstanceAttributefield and rewrite it. We can actually run the 7 replace commands and that should do the tricky.
|
@badra001, Let's also increase the MAX_API_RETRY from 10 to 15 in the ENV variable. I don't think it should add much longer to the overall run time, but in my testing, I saw it reaching up to 9 so it's possible that we may be close to the limit of 10. I think 15 will add potentially another 1 min to the run time possibly if needed. |
|
@badra001 and one more... enable DEBUG as well. |
|
It's already enabled as debug. This is deployed to the region, so it covers all VPCs in ma8-gov. I will do my instances testing in dev though. |
Issue 20
Issue 22
put_tem_in_dynamodb_table(line 279) portion to write the result after json.dumps instead of performing str(dictionary). This provides a more resilient writing and reading of the data.get_item_in_dynamodb_tablegrab the string and them perform json.loads. Also added 7 replace commands to handle any DDB item that were written prior to this change.