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

sagemaker: Some CfnFeatureGroup properties generate incorrect capitalization #29897

Open
adrianjhunter opened this issue Apr 19, 2024 · 3 comments
Labels
@aws-cdk/aws-sagemaker Related to AWS SageMaker bug This issue is a bug. effort/small Small work item – less than a day of effort p3

Comments

@adrianjhunter
Copy link

Describe the bug

I attempted to create a new CfnFeatureGroup with the OnlineStoreConfig and OfflineStoreConfig properties configured. When the template is synthesized, the resulting CFN template uses lower camel case for these specific properties while the CFN specification requires upper camel case.

Expected Behavior

Expecting output in upper camel case as per spec.

TestFeatureGroup:
    Type: AWS::SageMaker::FeatureGroup
    Properties:
      FeatureGroupName: test
      OfflineStoreConfig:
        S3StorageConfig:
          S3Uri:
            Fn::Join:
              - ""
              - - s3://
                - Ref: offlineStore4208FAB6
          KmsKeyId:
            Ref: kmsKey3BB021CC
        DataCatalogConfig:
          Catalog: AwsDataCatalog
          Database: testdb
          TableName: test
      OnlineStoreConfig:
        EnableOnlineStore: true
        SecurityConfig:
          KmsKeyId:
            Ref: kmsKey3BB021CC
      ...

Current Behavior

As show below, the CFN template that is outputted is in lower camel case for the OfflineStorageConfig, OnlineStoreConfig, OnlineStoreSecurityConfig, DataCatalogConfig and S3StorageConfig properties.

TestFeatureGroup:
    Type: AWS::SageMaker::FeatureGroup
    Properties:
      FeatureGroupName: test
      OfflineStoreConfig:
        s3StorageConfig:
          s3Uri:
            Fn::Join:
              - ""
              - - s3://
                - Ref: offlineStore4208FAB6
          kmsKeyId:
            Ref: kmsKey3BB021CC
        dataCatalogConfig:
          catalog: AwsDataCatalog
          database: testdb
          tableName: test
      OnlineStoreConfig:
        enableOnlineStore: true
        securityConfig:
          kmsKeyId:
            Ref: kmsKey3BB021CC
      ...

Reproduction Steps

Below is a snippet of some CDK resources which creates an S3 bucket, KMS key and a FeatureGroup referencing said bucket and key.

    const bucket = new s3.Bucket(this, 'offlineStore');
    const kms_key = new kms.Key(this, 'kmsKey');
    
    new sagemaker.CfnFeatureGroup(this, 'TestFeatureGroup', {
      featureGroupName: "test",
      eventTimeFeatureName: 'eventTime',
      recordIdentifierFeatureName: 'recordIdentifier',
      featureDefinitions: [
        {
          featureName: 'recordIdentifier',
          featureType: 'String'
        },
        {
          featureName: 'eventTime',
          featureType: 'String'
        }
      ],
      onlineStoreConfig: {
        enableOnlineStore: true,
        securityConfig: {
          kmsKeyId: kms_key.keyId,
        }
      },
      offlineStoreConfig: {
        s3StorageConfig: {
          s3Uri: bucket.s3UrlForObject(),
          kmsKeyId: kms_key.keyId,
        },
        dataCatalogConfig: {
          catalog: 'AwsDataCatalog',
          database: 'testdb',
          tableName: 'test'
        }
      },
    })

Possible Solution

Not sure where this error occurs, but originally discovered this using CDK Python before replicating in Typescript. So its pre the JSII step and might be related to the raw CFN import into L1 Constructs.

Additional Information/Context

No response

CDK CLI Version

2.138.0 (build 6b41c8b)

Framework Version

2.138.0

Node.js Version

v18.16.0

OS

MacOS 13.6.4

Language

TypeScript, Python

Language Version

No response

Other information

CFN doc of impacted properties found:

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-offlinestoreconfig.html
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-s3storageconfig.html
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-datacatalogconfig.html
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-onlinestoreconfig.html
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-onlinestoresecurityconfig.html

@adrianjhunter adrianjhunter added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 19, 2024
@github-actions github-actions bot added the @aws-cdk/aws-sagemaker Related to AWS SageMaker label Apr 19, 2024
@pahud
Copy link
Contributor

pahud commented Apr 22, 2024

They are actually untyped because of this patch to avoid breaking changes and you should see that from the IDE as below.

image

or from the API reference doc:

image

With the untyped properties, you will need to define JSON-like property and specify correct capitalization.

Check this out for more details about the patches #21767

Let me know if it helps for you.

@pahud pahud added p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. 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 Apr 22, 2024
@pahud pahud changed the title (aws-sagemaker): Some CfnFeatureGroup properties generate incorrect capitalization sagemaker: Some CfnFeatureGroup properties generate incorrect capitalization Apr 22, 2024
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Apr 24, 2024
@adrianjhunter
Copy link
Author

True, but if I use something like the OfflineStoreConfigProperty for instance (https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sagemaker.CfnFeatureGroup.OfflineStoreConfigProperty.html), it is typed but has the incorrect case.

For instance, this snippet of guidance from https://docs.aws.amazon.com/cdk/api/v2/python/aws_cdk.aws_sagemaker/CfnFeatureGroup.html#offlinestoreconfigproperty

image

Doesn't actually work as it generates incorrect case. The any type helps, e.g. if I ignore most of the properties under CfnFeatureGroup, I would be able to use the resource, but I feel like the properties should be corrected.

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Apr 25, 2024
@pahud pahud added p3 and removed p2 labels Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-sagemaker Related to AWS SageMaker bug This issue is a bug. effort/small Small work item – less than a day of effort p3
Projects
None yet
Development

No branches or pull requests

2 participants