iasql icon indicating copy to clipboard operation
iasql copied to clipboard

get rid of explicit memo/cache accesses wherever possible

Open depombo opened this issue 2 years ago • 1 comments

if the role doesn't exist in aws, every single apply/install it hits aws for every task definition with a non-existent role. if we have hundreds of them, which I ran into it broke locally. with a mappers.role.cloud.read(roleName, ctx) on every call, we are not resilient to certain installs. however, for the uninstallation and installation only the top level modules get loaded in the cache. dependencies don't get loaded so we have to check if the cache is loaded before a mappers.role.cloud.read(roleName, ctx) then

      if (td.taskRoleArn) {
        const roleName = AwsIamModule.utils.roleNameFromArn(td.taskRoleArn);
        // there can be hundreds of task defintions so don't do an aws call for each
        if (!Object.values(ctx.memo?.cloud?.Role ?? {}).length) {
          try {
            out.taskRole = await AwsIamModule.mappers.role.db.read(ctx, roleName) ??
            AwsIamModule.mappers.role.cloud.read(ctx, roleName);
          } catch (e) {
            // Role could have been deleted
            logger.error('Role not found', e as any);
            out.taskRole = undefined;
          }
        } else {
          out.taskRole = await AwsIamModule.mappers.role.db.read(ctx, roleName) ??
            ctx?.memo?.cloud?.Role?.[roleName ?? ''];
        }
      }

ideally we get rid of this logic in the module definition

depombo avatar Apr 07 '22 15:04 depombo

Concrete steps to tackle this:

  1. If we make the updateOrReplace function choose between an update function and a replace function, we can reduce the "branchiness" of the current update function that is the usual source of the need to get access to the memo. (This replace function could also have a default implementation so it isn't even necessary to specify most of the time, since it can just be a pair of delete and create calls.)
  2. If we add a third, optional argument for the existing cloud entities that these functions can just consume when needed, we eliminate most of the rest of the uses of memo.
  3. Adding a couple of functions to the Context object to let a method choose to bust the cache, or replace a cache value without actually mutating the database or cloud (used in one weird place in the security group mapper), we can have a typed mechanism for memo manipulation that eliminates the final usages.

dfellis avatar Sep 21 '22 23:09 dfellis