feat(location): support Tracker and TrackerConsumer #31268

mazyu36 · 2024-08-30T13:17:12Z

Issue # (if applicable)

Closes #30712.

Reason for this change

To support tracker and tracker consumer.

Description of changes

Add Tracker and TrackerConsumer class.

Description of how you validated changes

Add unit tests and integ tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

mazyu36 · 2024-09-03T12:21:07Z

packages/@aws-cdk/aws-location-alpha/lib/tracker.ts

+    });
+
+    props.geofenceCollections?.forEach((collection) => {
+      new CfnTrackerConsumer(this, `TrackerConsumer${collection.node.id}`, {


Initially, I had TrackerConsumer as a separate Construct.
5fb3cdd#diff-fc1da6063a6db97ebec475178eaaa40ce1a7d0a56f8c1b5cb249b99f33e9946a

However, since it only involved associations, I decided to integrate it into the Tracker.
If you have any opinions on this design, please let me know.

However, since it only involved associations, I decided to integrate it into the Tracker.
If you have any opinions on this design, please let me know.

@mazyu36 what do you mean by it only involved associations? Just looking to clarify my understanding. Here it seems CDK users would not be able to create TrackerConsumer objects via an L2 - does this make sense?

@sumupitchayan
TrackerConsumer is just an association between Tracker and Consumer (GeofenceCollection) and has no actual entity.

I thought the association could be auto-generated, similar to how VPC Subnet and route table associations work.
https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-ec2/lib/vpc.ts#L2117

Alternatively, TrackerConsumer could be made into a separate L2 construct.

Which approach would be better do you think?

@mazyu36 thanks for the clarification, this design makes sense to me. From my understanding, customers won't directly be creating TrackerConsumer objects and this case is similar to how VPC Subnets/route table associations are designed.

Additionally, since this is an alpha module and it will be easy to add an L2 for TrackerConsumer for this design, I think we can just do that in the future if we find it is needed for a customer use case.

sumupitchayan · 2024-10-16T18:38:37Z

packages/@aws-cdk/aws-location-alpha/test/integ.tracker.ts

+
+const app = new App();
+
+new integ.IntegTest(app, 'TrackerTest', {


Thanks for the PR @mazyu36 - I'm wondering if it's possible to add some integ test assertions here to validate that the construct works beyond deployment

Thank you. I added an assertion to ensure that the tracker consumers are correctly created.
How does this look?

@mazyu36 where is the assertion? I don't see it in this file

@sumupitchayan
The following line. Please let me know if this is insufficient.

aws-cdk/packages/@aws-cdk/aws-location-alpha/test/integ.tracker.ts

Line 52 in a0d0cd3

test.assertions.awsApiCall('location', 'ListTrackerConsumersCommand', { TrackerName: stack.tracker.trackerName })

Ah yes sorry I missed this for some reason - thanks for adding this! Approved 👍

mergify · 2024-10-28T16:04:00Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-10-28T16:40:53Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 0a190c6
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-10-28T16:41:50Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2024-10-28T16:42:06Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

aws-cdk-automation requested a review from a team August 30, 2024 13:17

github-actions bot added 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 labels Aug 30, 2024

aws-cdk-automation previously requested changes Aug 30, 2024

View reviewed changes

mazyu36 changed the title ~~WIP: feat(location): support Tracker and TrackerConsumer~~ feat(location): support Tracker and TrackerConsumer Aug 31, 2024

mazyu36 marked this pull request as ready for review August 31, 2024 02:10

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Aug 31, 2024

mazyu36 changed the title ~~feat(location): support Tracker and TrackerConsumer~~ WIP:feat(location): support Tracker and TrackerConsumer Aug 31, 2024

mazyu36 marked this pull request as draft August 31, 2024 02:46

aws-cdk-automation previously requested changes Aug 31, 2024

View reviewed changes

mazyu36 force-pushed the location-tracker-30712 branch from 442c737 to fb4d607 Compare August 31, 2024 08:59

add tracker

5fb3cdd

mazyu36 force-pushed the location-tracker-30712 branch from fb4d607 to 5fb3cdd Compare August 31, 2024 10:09

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Aug 31, 2024

Improve method for adding geofence collections

1f05756

mazyu36 force-pushed the location-tracker-30712 branch from a815c83 to 1f05756 Compare September 3, 2024 03:34

mazyu36 changed the title ~~WIP:feat(location): support Tracker and TrackerConsumer~~ feat(location): support Tracker and TrackerConsumer Sep 3, 2024

mazyu36 marked this pull request as ready for review September 3, 2024 03:35

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 3, 2024

mazyu36 commented Sep 3, 2024

View reviewed changes

sumupitchayan reviewed Oct 16, 2024

View reviewed changes

mazyu36 added 2 commits October 18, 2024 00:11

add assertion

9c1ba0d

Merge remote-tracking branch 'upstream/main' into location-tracker-30712

a0d0cd3

mazyu36 requested a review from sumupitchayan October 17, 2024 15:55

sumupitchayan approved these changes Oct 28, 2024

View reviewed changes

Merge branch 'main' into location-tracker-30712

0a190c6

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 28, 2024

mergify bot merged commit 046f041 into aws:main Oct 28, 2024
12 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2024

mazyu36 deleted the location-tracker-30712 branch October 28, 2024 22:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(location): support Tracker and TrackerConsumer #31268

feat(location): support Tracker and TrackerConsumer #31268

mazyu36 commented Aug 30, 2024 •

edited

Loading

aws-cdk-automation left a comment

aws-cdk-automation left a comment

mazyu36 Sep 3, 2024

sumupitchayan Oct 21, 2024

mazyu36 Oct 22, 2024

sumupitchayan Oct 28, 2024

sumupitchayan Oct 16, 2024

mazyu36 Oct 17, 2024

sumupitchayan Oct 28, 2024

mazyu36 Oct 28, 2024

sumupitchayan Oct 28, 2024

mergify bot commented Oct 28, 2024

aws-cdk-automation commented Oct 28, 2024

mergify bot commented Oct 28, 2024

github-actions bot commented Oct 28, 2024


		const app = new App();

		new integ.IntegTest(app, 'TrackerTest', {

feat(location): support Tracker and TrackerConsumer #31268

feat(location): support Tracker and TrackerConsumer #31268

Conversation

mazyu36 commented Aug 30, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Oct 28, 2024

aws-cdk-automation commented Oct 28, 2024

AWS CodeBuild CI Report

mergify bot commented Oct 28, 2024

github-actions bot commented Oct 28, 2024

mazyu36 commented Aug 30, 2024 •

edited

Loading