Skip to content

Support type 'list' for output_wrapper_field_path #1285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
brycahta opened this issue May 6, 2022 · 4 comments
Closed

Support type 'list' for output_wrapper_field_path #1285

brycahta opened this issue May 6, 2022 · 4 comments
Assignees
Labels
kind/enhancement Categorizes issue or PR as related to existing feature enhancements. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@brycahta
Copy link
Contributor

brycahta commented May 6, 2022

Is your feature request related to a problem?
I am unable to generate Instance CRD for ec2-controller because code-generator cannot unwrap the list responses from RunInstances and DescribeInstances, the Create and List operations, respectively.

RunInstances is a "create many" call in that a single request can create multiple instances. As a result, the output shape returns a list of Instances (within a Reservation struct). Similarly, DescribeInstances returns a list of Reservations each containing a list of Instances.

The Instance CRD implementation will only allow requesting/creating a single instance at a time; therefore, we only want the first instance in the RunInstances output shape, Reservation.Instances[0], and the first instance of the first reservation in the DescribeInstances output shape, Reservations[0].Instances[0].

Describe the solution you'd like
Extend the functionality of output_wrapper_field_path config to support type "list" (only supports struct today). Unwrapping a list element will always take the first element.

Desired generator.yaml for Instance:

  RunInstances:
    output_wrapper_field_path: Reservation.Instances
    operation_type:
      - Create
    resource_name: Instance
  DescribeInstances:
    output_wrapper_field_path: Reservations.Instances
    operation_type:
      - List
    resource_name: Instance
  • refactor setResourceReadMany to setListResource (or something similar) and use this function for ReadMany and unwrapping list elements.

Describe alternatives you've considered

  • map RunInstances to CreateBatch operation and create a setResourceCreateMany()
@brycahta brycahta added the kind/enhancement Categorizes issue or PR as related to existing feature enhancements. label May 6, 2022
@brycahta brycahta self-assigned this May 6, 2022
@jaypipes
Copy link
Collaborator

jaypipes commented May 6, 2022

@brycahta for the List operation, we have a ListOperationConfig.MatchNames thing that is supposed to handle this kind of problem for scenarios where the list operation returns a list of structs. I agree with you that we have no facility to handle scenarios where the list operation returns a list of structs of lists of structs :)

I think your proposal here is a good one.

@jaypipes
Copy link
Collaborator

jaypipes commented May 6, 2022

Basically, we want to make this piece of code configurable...

ack-bot pushed a commit to aws-controllers-k8s/code-generator that referenced this issue May 24, 2022
Issue #, if available: aws-controllers-k8s/community#1285

Description of changes:
* adds support for list types to `output_wrapper_field_path`
* cleans up logic in `setResourceReadMany`
* updates EC2 unit tests in model and code packages

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
@eks-bot
Copy link

eks-bot commented Aug 4, 2022

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/lifecycle stale

@ack-bot ack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 4, 2022
@a-hilaly
Copy link
Member

a-hilaly commented Aug 4, 2022

/lifecycle frozen

@ack-bot ack-bot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Categorizes issue or PR as related to existing feature enhancements. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

5 participants