-
Notifications
You must be signed in to change notification settings - Fork 29.1k
[SPARK-31533][SQL][TESTS] Enable DB2IntegrationSuite test and upgrade the DB2 docker inside #28325
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
Conversation
… the DB2 docker inside
| "LICENSE" -> "accept" | ||
| "LICENSE" -> "accept", | ||
| "DBNAME" -> "foo", | ||
| "ARCHIVE_LOGS" -> "false", |
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.
ARCHIVE_LOGS and AUTOCONFIG is needed because it triggers a server side restart (which made the suite fail all the time). Please see the following code snippet extracted from the docker image:
configure_db()
{
dbname=$1
if [ ${ARCHIVE_LOGS?} = "true" -o "${HADR_ENABLED?}" = "true" ]; then
# Enabling log archiving does not exit with a zero because of SQL1363W
enable_log_archiving ${dbname?}
restart_db2
if ! back_up ${dbname?}; then
echo "(!) Failed to back up ${dbname?} database"
fi
else
echo "(*) Log archiving will not be configured as ARCHIVE_LOGS has been set to false. "
fi
if [ ${AUTOCONFIG?} = "true" -o "${HADR_ENABLED?}" = "true" ]; then
if ! run_autoconfig ${dbname?}; then
echo "(!) Failed to automatically configure ${dbname?} database"
fi
else
echo "(*) Instance and database will not be auto configured. AUTOCONFIG has been set to false. "
fi
if ! configure_text_search ${dbname?}; then
echo "(!) Failed to configure ${dbname?} database for text search"
fi
}
I've applied the same in the kerberos suite because until now it was only timing luck that it's not failed.
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.
Got it.
|
cc @HeartSaVioR |
|
Test build #121747 has finished for PR 28325 at commit
|
|
retest this, please |
|
Test build #121760 has finished for PR 28325 at commit
|
|
retest this, please |
|
Thank you, @gaborgsomogyi . |
| ) | ||
| override val usesIpc = false | ||
| override val jdbcPort: Int = 50000 | ||
| override val privileged = true |
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.
Ur, why privileged is required?
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.
This docker requires priviliged run. Please see the how to use section of the docker image.
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.
Thanks. I checked the doc. It's at least required during re-mount the volumes even we don't provide a volume.
|
Test build #121771 has finished for PR 28325 at commit
|
dongjoon-hyun
left a comment
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.
+1, LGTM. Thank you. I verified this locally.
DB2IntegrationSuite:
...
- Basic test
- Numeric types
- Date types
- String types
- Basic write test
- query JDBC option
...
|
Merged to master. |
What changes were proposed in this pull request?
This is a followup PR discussed here.
Why are the changes needed?
It would be good to re-enable
DB2IntegrationSuiteand upgrade the docker image inside to use the latest.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing docker integration tests.