-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Some fixes and tests for spaces/non-ASCII chars in datasource names #6761
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
Changes from all commits
72a3e67
6e45599
6f0f16a
be39228
0877e4a
830b6c1
2206779
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,9 @@ public class DockerConfigProvider implements IntegrationTestingConfigProvider | |
| @NotNull | ||
| private String hadoopDir; | ||
|
|
||
| @JsonProperty | ||
| private String extraDatasourceNameSuffix = ""; | ||
|
|
||
| @Override | ||
| public IntegrationTestingConfig get() | ||
| { | ||
|
|
@@ -202,6 +205,12 @@ public boolean manageKafkaTopic() | |
| { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public String getExtraDatasourceNameSuffix() | ||
| { | ||
| return extraDatasourceNameSuffix; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have a use case to make the suffix configurable ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My thinking was that the non-ASCII characters could make the tests fail for people who do not have the proper locales set up on their system. I had to tweak locale settings on the ubuntu containers used in the integration tests (IT setup uses a shared folder too so the host machine needs to be configured as well), and people may not have any need for such characters in their own development/use cases, so I felt it would be nice to have a way to disable the use of such characters in the tests. Another reason for making it a "global" thing like this is because I wanted to easily test support across all the use cases being tested in the IT suite (e.g., if I wanted to test support for some other characters across the board, I can just edit this property instead of changing each test individually) |
||
| } | ||
| }; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also do a decode and verify that the original string is read back ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added decode checks