Skip to content

Conversation

badmintoncryer
Copy link
Contributor

@badmintoncryer badmintoncryer commented Oct 2, 2025

Issue # (if applicable)

Closes #35428.

Reason for this change

AWS Glue partition projection configuration was not type-safe, making it prone to runtime errors.
Specifically:

  • String-based configuration lacked type checking
  • Required properties differed by projection type, but this wasn't enforced by types
  • Validation errors were only detected at runtime

Description of changes

Refactored partition projection configuration to use a type-safe factory method pattern:

1. Factory Methods

  • PartitionProjectionConfiguration.integer() - INTEGER type projection
  • PartitionProjectionConfiguration.date() - DATE type projection
  • PartitionProjectionConfiguration.enum() - ENUM type projection
  • PartitionProjectionConfiguration.injected() - INJECTED type projection

Each factory method has a dedicated Props interface, with TypeScript's type system enforcing required
properties.

Describe any new or updated permissions being added

None

Description of how you validated changes

Add both unit and integ tests.

Checklist


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

@aws-cdk-automation aws-cdk-automation requested a review from a team October 2, 2025 15:18
@github-actions github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 distinguished-contributor [Pilot] contributed 50+ PRs to the CDK labels Oct 2, 2025
@badmintoncryer
Copy link
Contributor Author

badmintoncryer commented Oct 3, 2025

  • Do not use union
  • It is better to define PartitionProjection class and create factory methods such as PartitionProjection.Integer().
  • @ throws may not be needed
  • Do not import like readonly partitionProjection?: import('./partition-projection').PartitionProjection;
  • Unit test for enum definition may not be essential.
describe('DateIntervalUnit', () => {
  test('has correct enum values', () => {
    expect(glue.DateIntervalUnit.YEARS).toBe('YEARS');
    expect(glue.DateIntervalUnit.MONTHS).toBe('MONTHS');
    expect(glue.DateIntervalUnit.WEEKS).toBe('WEEKS');
    expect(glue.DateIntervalUnit.DAYS).toBe('DAYS');
    expect(glue.DateIntervalUnit.HOURS).toBe('HOURS');
    expect(glue.DateIntervalUnit.MINUTES).toBe('MINUTES');
    expect(glue.DateIntervalUnit.SECONDS).toBe('SECONDS');
  });

  test('has all 7 interval units', () => {
    const values = Object.values(glue.DateIntervalUnit);
    expect(values).toHaveLength(7);
    expect(values).toContain('YEARS');
    expect(values).toContain('MONTHS');
    expect(values).toContain('WEEKS');
    expect(values).toContain('DAYS');
    expect(values).toContain('HOURS');
    expect(values).toContain('MINUTES');
    expect(values).toContain('SECONDS');
  });
});

@badmintoncryer badmintoncryer marked this pull request as ready for review October 11, 2025 14:39
@xkjjx
Copy link

xkjjx commented Oct 13, 2025

Thanks for working on this @badmintoncryer !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

distinguished-contributor [Pilot] contributed 50+ PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cdk-glue-alpha: Have properties for setting partition projection

2 participants