Skip to content

fix event target id validation #7688

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
Merged
merged 8 commits into from
Feb 27, 2023
Merged

fix event target id validation #7688

merged 8 commits into from
Feb 27, 2023

Conversation

pinzon
Copy link
Member

@pinzon pinzon commented Feb 14, 2023 •
edited
Loading

This PR adds the validation for the Targets Ids in the Put Target Operation. Addresses #7567

Changes:

  • validation
  • tests

@pinzon pinzon temporarily deployed to localstack-ext-tests February 14, 2023 19:20 — with GitHub Actions Inactive
@pinzon pinzon temporarily deployed to localstack-ext-tests February 14, 2023 19:51 — with GitHub Actions Inactive
@pinzon pinzon marked this pull request as ready for review February 14, 2023 19:54
@github-actions
Copy link

github-actions bot commented Feb 14, 2023 •
edited
Loading

LocalStack integration with Pro

       3 files  ±  0         3 suites  ±0   1h 32m 56s ⏱️ -2s
1 756 tests +17  1 385 ✔️ +1  371 💤 +16  0 ❌ ±0 
2 474 runs  +17  1 761 ✔️ +1  713 💤 +16  0 ❌ ±0 

Results for commit 1815969. ± Comparison against base commit 8eaabbd.

♻️ This comment has been updated with latest results.

@pinzon pinzon temporarily deployed to localstack-ext-tests February 15, 2023 15:21 — with GitHub Actions Inactive
@pinzon pinzon temporarily deployed to localstack-ext-tests February 17, 2023 17:26 — with GitHub Actions Inactive
@pinzon pinzon temporarily deployed to localstack-ext-tests February 17, 2023 17:57 — with GitHub Actions Inactive
@pinzon pinzon force-pushed the feat-event_target_id_validation branch from e752bfe to 15ccb3f Compare February 22, 2023 20:15
@pinzon pinzon temporarily deployed to localstack-ext-tests February 22, 2023 20:15 — with GitHub Actions Inactive
@coveralls
Copy link

coveralls commented Feb 22, 2023 •
edited
Loading

Coverage Status

Coverage: 85.169% (+0.01%) from 85.158% when pulling 1815969 on feat-event_target_id_validation into 8eaabbd on master.

dominikschubert
dominikschubert requested changes Feb 23, 2023
View reviewed changes
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

Thanks for churning through these issues 🙏

Some concerns regarding validation & test cases, then good to go 👍

@pinzon @dominikschubert
Update tests/integration/test_events.py
47a4f85 
Co-authored-by: Dominik Schubert <dominik.schubert91@gmail.com>
@pinzon pinzon temporarily deployed to localstack-ext-tests February 24, 2023 15:35 — with GitHub Actions Inactive
@pinzon pinzon temporarily deployed to localstack-ext-tests February 24, 2023 17:25 — with GitHub Actions Inactive
@pinzon pinzon temporarily deployed to localstack-ext-tests February 24, 2023 19:59 — with GitHub Actions Inactive
dominikschubert
dominikschubert approved these changes Feb 27, 2023
View reviewed changes
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@pinzon pinzon merged commit 4914e36 into master Feb 27, 2023
@pinzon pinzon deleted the feat-event_target_id_validation branch February 27, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominikschubert dominikschubert dominikschubert approved these changes

@dfangl dfangl Awaiting requested review from dfangl

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pinzon @coveralls @dominikschubert