chore(rds): document DatabaseInstance.fromDatabaseInstanceAttributes() #11979

ktheory · 2020-12-10T07:57:30Z

Ensure DatabaseInstance.fromDatabaseInstanceAttributes is included in docs by moving it from the abstract DatabaseInstanceBase to the concrete DatabaseInstance class.

Thx @skinny85 for the suggestion in #11817.

Fixes #11817

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2020-12-10T07:57:33Z

@skinny85

…method Ensure `DatabaseInstance.fromDatabaseInstanceAttributes` is included in docs by moving it from the abstract `DatabaseInstanceBase` to the concrete `DatabaseInstance` class. Thx @skinny85 for the suggestion in aws#11817. Fixes aws#11817

fixes: ``` Member fromDatabaseInstanceAttributes should be declared before all field definitions @typescript-eslint/member-ordering ```

ktheory · 2020-12-10T13:51:18Z

From the build logs, looks like the error is about breaking backwards compatibility, which is strictly true. I'm curious if it matters in this case since DatabaseInstanceBase and intermediate classes are abstract.

Updated assembly:  @aws-cdk/[email protected]
API elements with incompatible changes:
err  - METHOD @aws-cdk/aws-rds.DatabaseInstanceBase.fromDatabaseInstanceAttributes: has been removed [removed:@aws-cdk/aws-rds.DatabaseInstanceBase.fromDatabaseInstanceAttributes]
err  - METHOD @aws-cdk/aws-rds.DatabaseInstanceFromSnapshot.fromDatabaseInstanceAttributes: has been removed [removed:@aws-cdk/aws-rds.DatabaseInstanceFromSnapshot.fromDatabaseInstanceAttributes]
err  - METHOD @aws-cdk/aws-rds.DatabaseInstanceReadReplica.fromDatabaseInstanceAttributes: has been removed [removed:@aws-cdk/aws-rds.DatabaseInstanceReadReplica.fromDatabaseInstanceAttributes]
...
Some packages seem to have undergone breaking API changes. Please avoid.

I could keep the method on DatabaseInstanceBase and mark it as deprecated to avoid breaking compatibility.
Curious for guidance, @skinny85.

skinny85

Looks great @ktheory , thanks for the contribution!

Let's appease the backwards-compatibility checker by leaving the existing method 🙂.

packages/@aws-cdk/aws-rds/lib/instance.ts

Pull request has been modified.

Preserve backwards compatibility. Deprecate and delegate to new method.

ktheory · 2020-12-11T12:21:15Z

@skinny85 just wanted to say kudos for making it easy and enjoyable to contribute to CDK.

In particular, I appreciate the effort y'all put in to the thorough Contributing guidelines, the helpful labels and comments on Issues, and robust CI+dev tooling.

skinny85

Thanks for the contribution @ktheory !

@skinny85 just wanted to say kudos for making it easy and enjoyable to contribute to CDK.

In particular, I appreciate the effort y'all put in to the thorough Contributing guidelines, the helpful labels and comments on Issues, and robust CI+dev tooling.

Thank you for the nice words, we're glad the contribution process is not too painful 😊.

mergify · 2020-12-15T01:17:15Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2020-12-15T01:46:28Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2020-12-15T01:48:08Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 1ade1b5
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…ributes() (#11979)" This reverts commit f2ecaf2.

@param

…() (#12091) Reverts #11979 This commit broke java compilation of the module: ```console #STDOUT> [ERROR] COMPILATION ERROR : #STDOUT> [INFO] ------------------------------------------------------------- #STDOUT> [ERROR] /tmp/npm-packP61iji/monocdk/src/main/java/software/amazon/awscdk/services/rds/DatabaseInstance.java:[38,5] method does not override or implement a method from a supertype ``` Basically the `fromDatabaseInstanceAttributes` function now exists both in `DatabaseInstanceBase` and `DatabaseInstance`, and for some reason the one in the child is annotated with an `@Override`, despite being a static function. (see more details below). Since this module is stable, I don't want to simply move the function, but I do want to bring our pipeline back to life quickly, so this seems like an ok compromise for now. ----------------- What happens is that the generated java code for some reason adds an `@Override` annotation to the function signature of the concrete class, as if its overriding the function in the base class, even though these are static methods. ```java @javax.annotation.Generated(value = "jsii-pacmak/1.15.0 (build 585166b)", date = "2020-12-15T17:40:10.678Z") @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable) @software.amazon.jsii.Jsii(module = software.amazon.awscdk.services.rds.$Module.class, fqn = "@aws-cdk/aws-rds.DatabaseInstance") public class DatabaseInstance extends software.amazon.awscdk.services.rds.DatabaseInstanceBase implements software.amazon.awscdk.services.rds.IDatabaseInstance { protected DatabaseInstance(final software.amazon.jsii.JsiiObjectRef objRef) { super(objRef); } protected DatabaseInstance(final software.amazon.jsii.JsiiObject.InitializationMode initializationMode) { super(initializationMode); } /** * @param scope This parameter is required. * @param id This parameter is required. * @param props This parameter is required. */ @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable) public DatabaseInstance(final @org.jetbrains.annotations.NotNull software.constructs.Construct scope, final @org.jetbrains.annotations.NotNull java.lang.String id, final @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.DatabaseInstanceProps props) { super(software.amazon.jsii.JsiiObject.InitializationMode.JSII); software.amazon.jsii.JsiiEngine.getInstance().createNewObject(this, new Object[] { java.util.Objects.requireNonNull(scope, "scope is required"), java.util.Objects.requireNonNull(id, "id is required"), java.util.Objects.requireNonNull(props, "props is required") }); } /** * Import an existing database instance. * <p> * @param scope This parameter is required. * @param id This parameter is required. * @param attrs This parameter is required. */ @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable) @OverRide public static @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.IDatabaseInstance fromDatabaseInstanceAttributes(final @org.jetbrains.annotations.NotNull software.constructs.Construct scope, final @org.jetbrains.annotations.NotNull java.lang.String id, final @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.DatabaseInstanceAttributes attrs) { return software.amazon.jsii.JsiiObject.jsiiStaticCall(software.amazon.awscdk.services.rds.DatabaseInstance.class, "fromDatabaseInstanceAttributes", software.amazon.jsii.NativeType.forClass(software.amazon.awscdk.services.rds.IDatabaseInstance.class), new Object[] { java.util.Objects.requireNonNull(scope, "scope is required"), java.util.Objects.requireNonNull(id, "id is required"), java.util.Objects.requireNonNull(attrs, "attrs is required") }); } ``` JSII assembly also shows: ```json "name": "fromDatabaseInstanceAttributes", "overrides": "@aws-cdk/aws-rds.DatabaseInstanceBase", "parameters": [ { "name": "scope", "type": { "fqn": "constructs.Construct" } }, { "name": "id", "type": { "primitive": "string" } }, { "name": "attrs", "type": { "fqn": "@aws-cdk/aws-rds.DatabaseInstanceAttributes" } } ], "returns": { "type": { "fqn": "@aws-cdk/aws-rds.IDatabaseInstance" } }, "static": true ``` This feels like a JSII bug, it shouldn't be marking this method as an override in the assembly, and regardless, it should not generate java code that has an `@Override` annotation on a static function, better to validate and throw beforehand. I guess this is related to Typescript quirks with inheritance of static members? cc @RomainMuller @skinny85

@skinny85

aws#11979) Ensure `DatabaseInstance.fromDatabaseInstanceAttributes` is included in docs by moving it from the abstract `DatabaseInstanceBase` to the concrete `DatabaseInstance` class. Thx @skinny85 for the suggestion in aws#11817. Fixes aws#11817 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

@param

…() (aws#12091) Reverts aws#11979 This commit broke java compilation of the module: ```console #STDOUT> [ERROR] COMPILATION ERROR : #STDOUT> [INFO] ------------------------------------------------------------- #STDOUT> [ERROR] /tmp/npm-packP61iji/monocdk/src/main/java/software/amazon/awscdk/services/rds/DatabaseInstance.java:[38,5] method does not override or implement a method from a supertype ``` Basically the `fromDatabaseInstanceAttributes` function now exists both in `DatabaseInstanceBase` and `DatabaseInstance`, and for some reason the one in the child is annotated with an `@Override`, despite being a static function. (see more details below). Since this module is stable, I don't want to simply move the function, but I do want to bring our pipeline back to life quickly, so this seems like an ok compromise for now. ----------------- What happens is that the generated java code for some reason adds an `@Override` annotation to the function signature of the concrete class, as if its overriding the function in the base class, even though these are static methods. ```java @javax.annotation.Generated(value = "jsii-pacmak/1.15.0 (build 585166b)", date = "2020-12-15T17:40:10.678Z") @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable) @software.amazon.jsii.Jsii(module = software.amazon.awscdk.services.rds.$Module.class, fqn = "@aws-cdk/aws-rds.DatabaseInstance") public class DatabaseInstance extends software.amazon.awscdk.services.rds.DatabaseInstanceBase implements software.amazon.awscdk.services.rds.IDatabaseInstance { protected DatabaseInstance(final software.amazon.jsii.JsiiObjectRef objRef) { super(objRef); } protected DatabaseInstance(final software.amazon.jsii.JsiiObject.InitializationMode initializationMode) { super(initializationMode); } /** * @param scope This parameter is required. * @param id This parameter is required. * @param props This parameter is required. */ @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable) public DatabaseInstance(final @org.jetbrains.annotations.NotNull software.constructs.Construct scope, final @org.jetbrains.annotations.NotNull java.lang.String id, final @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.DatabaseInstanceProps props) { super(software.amazon.jsii.JsiiObject.InitializationMode.JSII); software.amazon.jsii.JsiiEngine.getInstance().createNewObject(this, new Object[] { java.util.Objects.requireNonNull(scope, "scope is required"), java.util.Objects.requireNonNull(id, "id is required"), java.util.Objects.requireNonNull(props, "props is required") }); } /** * Import an existing database instance. * <p> * @param scope This parameter is required. * @param id This parameter is required. * @param attrs This parameter is required. */ @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable) @OverRide public static @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.IDatabaseInstance fromDatabaseInstanceAttributes(final @org.jetbrains.annotations.NotNull software.constructs.Construct scope, final @org.jetbrains.annotations.NotNull java.lang.String id, final @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.DatabaseInstanceAttributes attrs) { return software.amazon.jsii.JsiiObject.jsiiStaticCall(software.amazon.awscdk.services.rds.DatabaseInstance.class, "fromDatabaseInstanceAttributes", software.amazon.jsii.NativeType.forClass(software.amazon.awscdk.services.rds.IDatabaseInstance.class), new Object[] { java.util.Objects.requireNonNull(scope, "scope is required"), java.util.Objects.requireNonNull(id, "id is required"), java.util.Objects.requireNonNull(attrs, "attrs is required") }); } ``` JSII assembly also shows: ```json "name": "fromDatabaseInstanceAttributes", "overrides": "@aws-cdk/aws-rds.DatabaseInstanceBase", "parameters": [ { "name": "scope", "type": { "fqn": "constructs.Construct" } }, { "name": "id", "type": { "primitive": "string" } }, { "name": "attrs", "type": { "fqn": "@aws-cdk/aws-rds.DatabaseInstanceAttributes" } } ], "returns": { "type": { "fqn": "@aws-cdk/aws-rds.IDatabaseInstance" } }, "static": true ``` This feels like a JSII bug, it shouldn't be marking this method as an override in the assembly, and regardless, it should not generate java code that has an `@Override` annotation on a static function, better to validate and throw beforehand. I guess this is related to Typescript quirks with inheritance of static members? cc @RomainMuller @skinny85

github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Dec 10, 2020

github-actions bot assigned skinny85 Dec 10, 2020

ktheory force-pushed the ktheory/11817 branch from 6f12b66 to 243076f Compare December 10, 2020 08:01

ktheory changed the title ~~fix(rds): document DatabaseInstance.fromDatabaseInstanceAttributes()~~ chore(rds): document DatabaseInstance.fromDatabaseInstanceAttributes() Dec 10, 2020

ktheory and others added 2 commits December 10, 2020 03:30

Merge branch 'master' into ktheory/11817

32a8cb2

Fix linter

4f62b0c

fixes: ``` Member fromDatabaseInstanceAttributes should be declared before all field definitions @typescript-eslint/member-ordering ```

skinny85 previously requested changes Dec 10, 2020

View reviewed changes

packages/@aws-cdk/aws-rds/lib/instance.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-rds/lib/instance.ts Show resolved Hide resolved

add back DatabaseInstanceBase.fromDatabaseInstanceAttributes

6ff67c5

Preserve backwards compatibility. Deprecate and delegate to new method.

ktheory force-pushed the ktheory/11817 branch from 1498e5c to 6ff67c5 Compare December 11, 2020 02:55

skinny85 approved these changes Dec 15, 2020

View reviewed changes

Merge branch 'master' into ktheory/11817

1ade1b5

mergify bot merged commit f2ecaf2 into aws:master Dec 15, 2020

iliapolo added a commit that referenced this pull request Dec 15, 2020

Revert "chore(rds): document DatabaseInstance.fromDatabaseInstanceAtt…

18bf6bb

…ributes() (#11979)" This reverts commit f2ecaf2.

iliapolo mentioned this pull request Dec 15, 2020

revert(rds): document DatabaseInstance.fromDatabaseInstanceAttributes() #12091

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(rds): document DatabaseInstance.fromDatabaseInstanceAttributes() #11979

chore(rds): document DatabaseInstance.fromDatabaseInstanceAttributes() #11979

ktheory commented Dec 10, 2020

gitpod-io bot commented Dec 10, 2020 •

edited

Loading

ktheory commented Dec 10, 2020

skinny85 left a comment

ktheory commented Dec 11, 2020

skinny85 left a comment

mergify bot commented Dec 15, 2020

mergify bot commented Dec 15, 2020

aws-cdk-automation commented Dec 15, 2020

chore(rds): document DatabaseInstance.fromDatabaseInstanceAttributes() #11979

chore(rds): document DatabaseInstance.fromDatabaseInstanceAttributes() #11979

Conversation

ktheory commented Dec 10, 2020

gitpod-io bot commented Dec 10, 2020 • edited Loading

ktheory commented Dec 10, 2020

skinny85 left a comment

Choose a reason for hiding this comment

ktheory commented Dec 11, 2020

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 15, 2020

mergify bot commented Dec 15, 2020

aws-cdk-automation commented Dec 15, 2020

AWS CodeBuild CI Report

gitpod-io bot commented Dec 10, 2020 •

edited

Loading