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.

Sign up for GitHub

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

Jump to bottom

(@aws-cdk/aws-glue-alpha): New Prefix check logic is not follow normal S3 prefix format #27396

Closed
yuntaoL opened this issue Oct 3, 2023 · 2 comments · Fixed by #27472
Closed

(@aws-cdk/aws-glue-alpha): New Prefix check logic is not follow normal S3 prefix format #27396

yuntaoL opened this issue Oct 3, 2023 · 2 comments · Fixed by #27472
Assignees
@mikewrighton
Labels
@aws-cdk/aws-glue Related to AWS Glue bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@yuntaoL
Copy link

yuntaoL commented Oct 3, 2023

Describe the bug

The latest version of job.ts in @aws-cdk/aws-glue-alpha package added check on prefix, and require prefix must begin with "/" and not end with "/"
https://github.com/aws/aws-cdk/blame/0bb49b42406b9b3692063537e6194af80d65c7f9/packages/%40aws-cdk/aws-glue-alpha/lib/job.ts#L847
But, normally for S3 prefix, it should not start with "/" and for the folder, it should end with "/",
https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-prefixes.html

Expected Behavior

The prefix validation should check prefix must end with "/" not start with "/"

Current Behavior

        throw new Error(`Invalid prefix format (value: ${prefix})${os_1.EOL}${errors.join(os_1.EOL)}`);
        ^

Error: Invalid prefix format (value: sparkHistoryLogs/CxRfMtrc_RefinementMetric/)
Prefix must begin with '/'
Prefix must not end with '/'
at Job.validatePrefix (/Volumes/workplace/MarcaGlueDataProcessing/src/MarcaGlueDataProcessingCDK/node_modules/@aws-cdk/aws-glue-alpha/lib/job.js:451:19)

Reproduction Steps

Create Job construct with "/' ended sparkHistoryLogs parameter

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.99.0-alpha.0

Framework Version

No response

Node.js Version

18.0

OS

Macos

Language

Typescript

Language Version

No response

Other information

No response
@yuntaoL yuntaoL added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 3, 2023
@github-actions github-actions bot added the @aws-cdk/aws-glue Related to AWS Glue label Oct 3, 2023
@yuntaoL yuntaoL changed the title (module name): (short issue description) (@aws-cdk/aws-glue-alpha): (New Prefix check log is not follow normal S3 prefix format) Oct 3, 2023
@yuntaoL yuntaoL changed the title (@aws-cdk/aws-glue-alpha): (New Prefix check log is not follow normal S3 prefix format) (@aws-cdk/aws-glue-alpha): New Prefix check log is not follow normal S3 prefix format Oct 3, 2023
@yuntaoL yuntaoL changed the title (@aws-cdk/aws-glue-alpha): New Prefix check log is not follow normal S3 prefix format (@aws-cdk/aws-glue-alpha): New Prefix check logic is not follow normal S3 prefix format Oct 3, 2023
@scanlonp
Copy link
Contributor

scanlonp commented Oct 3, 2023

Thanks for the docs link. You are right, I can fix this.

@scanlonp scanlonp self-assigned this Oct 3, 2023
@scanlonp scanlonp added 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 Oct 3, 2023
@mikewrighton mikewrighton assigned mikewrighton and unassigned scanlonp Oct 5, 2023
@mikewrighton mikewrighton mentioned this issue Oct 9, 2023
Merged
@mergify mergify bot closed this as completed in #27472 Oct 10, 2023
mergify bot pushed a commit that referenced this issue Oct 10, 2023
@mikewrighton
fix(glue-alpha): prefix validation logic is incorrect (#27472)
b898d3b 
There was logic in `aws-glue-alpha.Job` that expected `SparkUIProps.prefix` to be specified in the form `/prefix-name`, instead of the more conventional `prefix-name/`. This fix addresses the validation and handling of this prefix field to support the correct format. 


BREAKING CHANGE: `SparkUIProps.prefix` strings in the original `/prefix-name` format will now result in a validation error. To retain the same behavior, prefixes must be changed to the new `prefix-name/` format.

Closes #27396.

----

*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.

@github-actions github-actions bot mentioned this issue Nov 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mikewrighton mikewrighton

Labels
@aws-cdk/aws-glue Related to AWS Glue bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(glue-alpha): prefix validation logic is incorrect aws/aws-cdk
3 participants
@yuntaoL @mikewrighton @scanlonp