Innablr / revolver

AWS Powercycle Facility
MIT License
2 stars 3 forks source link

Improve type-safety of pagination function #90

Open lyricnz opened 8 months ago

lyricnz commented 8 months ago

https://github.com/Innablr/revolver/blob/develop/lib/common.ts#L1

async function paginateAwsCall(paginatorRef: any, client: any, what: string, params?: any) {
  // TODO: improve parameter typing:
  //   paginatorRef = f(PaginationConfiguration, request)
  //   client = Client
  const parameters = params || {};
  const paginator = paginatorRef({client: client}, parameters)
  let entityList: any[] = [];
  for await (const page of paginator) {
    entityList.push(...page[what])
  }
  return entityList;
}

@alutman-innablr

Pagination follows a pretty similar structure paginateDescribeTags: (config: EC2PaginationConfiguration, input: DescribeTagsCommandInput, ...rest: any[]) => Paginator; might be able to lock down a bit with pageable(config: any, input: any) => Paginator

lyricnz commented 8 months ago

FWIW, using a generic that provides a concrete return-type:

async function paginateAwsCallT<T>(paginatorRef: any, client: any, what: string, params?: any): Promise<T[]> {
  const parameters = params || {};
  const paginator = paginatorRef({ client: client }, parameters);
  const entityList: T[] = [];
  for await (const page of paginator) {
    entityList.push(...page[what]);
  }
  return entityList;
}

This creates numerous errors with optional fields:

drivers/ec2.ts:198:46 - error TS18048: 'xi' is possibly 'undefined'.
198       if (inoperableStates.find((x) => x === xi.State.Name)) {
                                                 ~~

drivers/ec2.ts:198:46 - error TS18048: 'xi.State' is possibly 'undefined'.
198       if (inoperableStates.find((x) => x === xi.State.Name)) {
                                                 ~~~~~~~~

As well as errors when the result items have arbitrary data added to them (eg: adding instanceDetails to Volume, adding AutoScalingGroupName to Instances, etc)

        const accounts = await paginateAwsCallT<Account>(paginateListAccounts, client, 'Accounts');
        accounts.forEach((account) => {
          account.accountId = account.Id;
lib/config.ts:57:19 - error TS2339: Property 'accountId' does not exist on type 'Account'.
57           account.accountId = account.Id;
lyricnz commented 8 months ago

@abukharov @alutman-innablr is there any value in this? Or leave it loosey-goosey?

simon-anz commented 8 months ago

FWIW, this lack of type-safety hid a little bug where InstrumentedEc2() was constructed with a CreateVolumeCommandOutput rather than an Instance.