Skip to content
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

(aws-docdb): Incorrect enum in sample snippet #20747

Closed
greg5123334 opened this issue Jun 15, 2022 · 5 comments · Fixed by #20906
Closed

(aws-docdb): Incorrect enum in sample snippet #20747

greg5123334 opened this issue Jun 15, 2022 · 5 comments · Fixed by #20906
Assignees
Labels
@aws-cdk/aws-docdb Related to Amazon DocumentDB documentation This is a problem with documentation. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@greg5123334
Copy link
Contributor

Describe the issue

The API documentation for aws_docdb.DatabaseCluster, for CDKv1 and CDKv2 shares the following sample code:

cluster = docdb.DatabaseCluster(self, "Database",
    master_user=docdb.Login(
        username="myuser",  # NOTE: 'admin' is reserved by DocumentDB
        exclude_characters=""@/:",  # optional, defaults to the set ""@/" and is also used for eventually created rotations
        secret_name="/myapp/mydocdb/masteruser"
    ),
    instance_type=ec2.InstanceType.of(ec2.InstanceClass.R5, ec2.InstanceSize.LARGE),
    vpc_subnets=ec2.SubnetSelection(
        subnet_type=ec2.SubnetType.PUBLIC
    ),
    vpc=vpc
)

Deploying returns the following error, for both CDKv1 and v2:

Traceback (most recent call last):
  File "................\lab\app.py", line 10, in <module>
    LabStack(app, "LabStack",
  File "................\python\current\lib\site-packages\jsii\_runtime.py", line 86, in __call__
    inst = super().__call__(*args, **kwargs)
  File "................\lab\lab\lab_stack.py", line 21, in __init__
    instance_type=ec2.InstanceType.of(ec2.InstanceClass.R5, ec2.InstanceSize.LARGE),
  File "................\python\current\lib\enum.py", line 437, in __getattr__
    raise AttributeError(name) from None
AttributeError: R5

Subprocess exited with error 1

However, the following code works just fine:

    db_class_type = ec2.InstanceClass.MEMORY5
    db_class_size = ec2.InstanceSize.LARGE

    vpc = ec2.Vpc(self, "VPC")
    cluster = docdb.DatabaseCluster(self, "Database",
        master_user=docdb.Login(
        username="myuser"
    ),
    instance_type=ec2.InstanceType.of(db_class_type, db_class_size),
    vpc_subnets=ec2.SubnetSelection(
        subnet_type=ec2.SubnetType.PUBLIC
    ),
    vpc=vpc
    )

And synthesizes the following template:

    "DatabaseInstance1844F58FD": {
      "Type": "AWS::DocDB::DBInstance",
      "Properties": {
       "DBClusterIdentifier": {
        "Ref": "DatabaseB269D8BB"
       },
       "DBInstanceClass": "db.r5.large"
      },

One will notice the InstanceClass gets resolved to a R5 Large anyway. In accordance with the enum value.

While I am not quite sure why the R5 enum fails to work, despite it existing, the point is that to avoid such confusion for others, should we perhaps edit the documentation to reflect the MEMORY5 enum rather?

Links

https://docs.aws.amazon.com/cdk/api/v1/python/aws_cdk.aws_docdb/DatabaseCluster.html
https://docs.aws.amazon.com/cdk/api/v2/python/aws_cdk.aws_docdb/DatabaseCluster.html

@greg5123334 greg5123334 added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Jun 15, 2022
@github-actions github-actions bot added the @aws-cdk/aws-docdb Related to Amazon DocumentDB label Jun 15, 2022
@peterwoodworth
Copy link
Contributor

This is a known bug with our non - typescript languages. Enums that have multiple of the same value will be dropped to only include one. In this case, you've found that MEMORY5 is the same! So you'll have to stick with that one until we get this bug fixed.

We should update this example to use the enum that actually compiles to our other languages

@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 16, 2022
@greg5123334
Copy link
Contributor Author

ah thanks for the confirmation @peterwoodworth , good to know and much appreciated!! feel free to close this ticket when appropriate.

@peterwoodworth peterwoodworth self-assigned this Jun 20, 2022
@peterwoodworth
Copy link
Contributor

Hey @josephedward !

Our documentation is autogenerated from our TypeScript library - so you will only have to change the examples found in CDK TS code library. In this case, I can see three examples which use R5 in this readme!

@josephedward
Copy link
Contributor

Thanks @peterwoodworth, created a PR here #20906 - is there a way to run that transpiler locally to see it in action? Just curious 🤔

@mergify mergify bot closed this as completed in #20906 Jun 30, 2022
mergify bot pushed a commit that referenced this issue Jun 30, 2022
…ng used (#20906)

Due to a bug in jsii, only the first instance of enums that have duplicate values appear in non-TS languages. This means that non-TS examples that use the enums that do not appear fail to build. This PR changes the enums used to fix the failing examples.

fixes #20747

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

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

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

daschaa pushed a commit to daschaa/aws-cdk that referenced this issue Jul 9, 2022
…ng used (aws#20906)

Due to a bug in jsii, only the first instance of enums that have duplicate values appear in non-TS languages. This means that non-TS examples that use the enums that do not appear fail to build. This PR changes the enums used to fix the failing examples.

fixes aws#20747

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-docdb Related to Amazon DocumentDB documentation This is a problem with documentation. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants