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

chore(kinesisfirehose-destinations): refactor logging to combine logGroup and logging properties into loggingConfig #31488

Merged
merged 8 commits into from
Sep 25, 2024
Merged
Prev Previous commit
Next Next commit
remove comments and update docstrings
paulhcsun committed Sep 19, 2024
commit 408bbb171eb37809d8164c5a3271f43f36f75c5e
Original file line number Diff line number Diff line change
@@ -5,9 +5,6 @@ import * as logs from 'aws-cdk-lib/aws-logs';
*
* This class defines whether logging is enabled or disabled and holds
* the CloudWatch log group where error logs are stored if logging is enabled.
*
* Subclasses must implement whether logging is enabled (`EnableLogging`)
* or disabled (`DisableLogging`).
*/
export abstract class LoggingConfig {
/**
@@ -53,6 +50,5 @@ export class DisableLogging extends LoggingConfig {

constructor() {
super();
// No logGroup should be allowed when logging is disabled
}
}
}
Original file line number Diff line number Diff line change
@@ -200,14 +200,6 @@ describe('S3 destination', () => {
});
});

// it('throws error if logging disabled but log group provided', () => {
// const destination = new firehosedestinations.S3Bucket(bucket, { logging: false, logGroup: new logs.LogGroup(stack, 'Log Group') });

// expect(() => new firehose.DeliveryStream(stack, 'DeliveryStream', {
// destinations: [destination],
// })).toThrowError('logging cannot be set to false when logGroup is provided');
// });

it('grants log group write permissions to destination role', () => {
const logGroup = new logs.LogGroup(stack, 'Log Group');

@@ -572,8 +564,6 @@ describe('S3 destination', () => {
bufferingInterval: cdk.Duration.minutes(1),
compression: firehosedestinations.Compression.ZIP,
encryptionKey: key,
// logging: true,
// logGroup: logGroup,
loggingConfig: new firehosedestinations.EnableLogging(logGroup),
},
});