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.

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

Fix: externalID-white-spaces #514

Merged
merged 1 commit into from
Oct 19, 2022
Merged

Fix: externalID-white-spaces #514

merged 1 commit into from
Oct 19, 2022

Conversation

samuel-br
Copy link
Contributor

Issue & Steps to Reproduce / Feature Request

fixes #513

Solution

remove white space from every value assign to External id

@samuel-br samuel-br changed the title Fix-externalID-white-spaces Fix: externalID-white-spaces Oct 18, 2022
@samuel-br
Copy link
Contributor Author

Change TF provider integration test when value contain white space is assigned to AWS 'External Id'

@TomerHeber
Copy link
Collaborator

@samuel-br - just so I understand:
External ID with whitespaces is valid. But on your code, you added something that will throw an error if there is a whitespace?

@samuel-br
Copy link
Contributor Author

AWS wont allowed white spaces, we add this validation today to the backend, what makes the tests fail

@TomerHeber
Copy link
Collaborator

AWS wont allowed white spaces, we add this validation today to the backend, what makes the tests fail

@samuel-br - Understood. should it be validated in SDK as well. WDYT?

@samuel-br
Copy link
Contributor Author

AWS wont allowed white spaces, we add this validation today to the backend, what makes the tests fail

@samuel-br - Understood. should it be validated in SDK as well. WDYT?

For what I know, @yaronya planed to, so I belive ticket will be add next day

TomerHeber
TomerHeber approved these changes Oct 18, 2022
View reviewed changes
Copy link
Collaborator

@TomerHeber TomerHeber left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions github-actions bot added ready to merge PR approved - can be merged once the PR owner is ready and removed pending final review labels Oct 18, 2022
@samuel-br samuel-br merged commit e898a81 into main Oct 19, 2022
@samuel-br samuel-br deleted the Fix-AWS_externalId-whitespace-integration-test branch October 19, 2022 06:53
@yaronya
Copy link
Contributor

yaronya commented Oct 19, 2022

For what I know, @yaronya planed to, so I belive ticket will be add next day

@samuel-br Please add this as a GH issue

tomer-landesman
tomer-landesman reviewed Oct 19, 2022
View reviewed changes
Copy link
Contributor

@tomer-landesman tomer-landesman left a comment

Choose a reason for hiding this comment

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

lgtm

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

@tomer-landesman tomer-landesman tomer-landesman left review comments

@TomerHeber TomerHeber TomerHeber approved these changes

@HeverFarber HeverFarber Awaiting requested review from HeverFarber

@roni-frantchi roni-frantchi Awaiting requested review from roni-frantchi

Assignees

@samuel-br samuel-br

Labels
fix integration-tests ready to merge PR approved - can be merged once the PR owner is ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Integration tests assign values with white spaces to AWS 'External Id' property
4 participants
@samuel-br @TomerHeber @yaronya @tomer-landesman