Skip to content

Conversation

rajdivya
Copy link
Contributor

Issue #, if available:

Description of changes:
Add TypeConfiguration support for Resource Types

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

@rajdivya rajdivya marked this pull request as ready for review June 23, 2021 03:59
Copy link
Contributor

@ammokhov ammokhov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this must be merged befor #369

Copy link
Contributor

@ammokhov ammokhov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz fix coverage

@rajdivya
Copy link
Contributor Author

Failing checks are due to unreleased changes in cfn-cli. Validated that they pass locally.

➜  cloudformation-cli-java-plugin git:(typeConfig) pre-commit run --all-files

[WARNING] The 'rev' field of repo 'https://github.com/ambv/black' appears to be a mutable reference (moving tag / branch).  Mutable references are never updated after first install and are not supported.  See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details.  Hint: `pre-commit autoupdate` often fixes this.
isort....................................................................Passed
black....................................................................^[[1;9CPassed
Check for case conflicts.................................................Passed
Fix End of Files.........................................................Passed
Mixed line ending........................................................Passed
Trim Trailing Whitespace.................................................Passed
Flake8...................................................................Passed
Pretty format JSON.......................................................Passed
Check for merge conflicts................................................Passed
Check Yaml...............................................................Passed
Check blanket noqa.......................................................Passed
check for not-real mock methods..........................................Passed
use logger.warning(......................................................Passed
bandit...................................................................Passed
pylint-local.............................................................Passed
pytest-local.............................................................Passed

@rajdivya
Copy link
Contributor Author

Validated type configuration changes manually by running cfn-init, cfn-generate and cfn-validate.

Copy link
Contributor

@omkhegde omkhegde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the plugin version bump from this PR to maintain clean history

Comment on lines +84 to +87
public <ResourceT, CallbackT,
ConfigurationT> void rescheduleAfterMinutes(final String functionArn,
final int minutesFromNow,
final HandlerRequest<ResourceT, CallbackT, ConfigurationT> handlerRequest) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it even used anywhere? @srujithpoondla03

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I think there is some cleanup required in this repo for V1 Code(not just this pull request). We did not totally remove the code related to CW events yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants