Skip to content

Stop testing compatibility against Databricks 7.3#18538

Merged
ebyhr merged 2 commits intotrinodb:masterfrom
findinpath:findinpath/delta-stop-testing-databricks73
Aug 31, 2023
Merged

Stop testing compatibility against Databricks 7.3#18538
ebyhr merged 2 commits intotrinodb:masterfrom
findinpath:findinpath/delta-stop-testing-databricks73

Conversation

@findinpath
Copy link
Copy Markdown
Contributor

@findinpath findinpath commented Aug 4, 2023

Description

Databricks 7.3 is reaching on September 2023 end of support.

Release notes

(x) This is not user-visible or docs only and no release notes are required.

@cla-bot cla-bot bot added the cla-signed label Aug 4, 2023
@findinpath findinpath self-assigned this Aug 4, 2023
@findinpath findinpath added test no-release-notes This pull request does not require release notes entry delta-lake Delta Lake connector and removed tests:hive labels Aug 4, 2023
@findinpath findinpath force-pushed the findinpath/delta-stop-testing-databricks73 branch from 7bd99a9 to 7aa6790 Compare August 4, 2023 09:40
@findinpath findinpath force-pushed the findinpath/delta-stop-testing-databricks73 branch from 7aa6790 to 6dd99c5 Compare August 4, 2023 09:41
@findepi
Copy link
Copy Markdown
Member

findepi commented Aug 4, 2023

some checks have failed

Copy link
Copy Markdown
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

% CI

Copy link
Copy Markdown
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

Please remove 7.3 LTS from delta-lake.rst:

* Tables written by Databricks Runtime 7.3 LTS, 9.1 LTS, 10.4 LTS, 11.3 LTS, and
  12.2 LTS are supported.

@findinpath findinpath force-pushed the findinpath/delta-stop-testing-databricks73 branch from 6dd99c5 to 5ab7b0f Compare August 7, 2023 04:12
@github-actions github-actions bot added the docs label Aug 7, 2023
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

While at it, would it make sense to capture some tables created by 7.3 as static test resources and run tests against them?

Then we wouldn't change this file. Of course it's less about changing this file, but more about staying compatible with data out there. We should be able to read any Delta table, including those written by DBR << 7.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

BTW, I think most of the initial tables from the test resources used for testing trino-delta-lake are actually created with Databricks 7.3.

Trino can likely read as well tables written with non-LTS version of Databricks, but we're not mentioning this here.
I think there's no loss / misleading information if we remove 7.3 LTS from the documentation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I care less about docs, and more about testing coverage.
Tables created by DBR 7.3 do exist in the world and we must keep supporting them.
Is this test covered?

@findinpath findinpath force-pushed the findinpath/delta-stop-testing-databricks73 branch from 5ab7b0f to ca1b3e7 Compare August 11, 2023 11:34
@findinpath
Copy link
Copy Markdown
Contributor Author

@findepi please run the PR with secrets (I've slipped in a change which enables an existing product tests).

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Aug 14, 2023

/test-with-secrets sha=ca1b3e7f8795a7c59d60b2bb41f2aabc3ed65069

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 14, 2023

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/5850234042

@findinpath
Copy link
Copy Markdown
Contributor Author

I guess that the failure pt (default, suite-delta-lake-databricks73, ) has to do with the fact that test-with-secrets functionality doesn't cope well with changes on ci.yml file.

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Aug 14, 2023

Sent #18665 within repository.

@findinpath findinpath force-pushed the findinpath/delta-stop-testing-databricks73 branch from ca1b3e7 to 7cb65b2 Compare August 30, 2023 16:19
@findinpath
Copy link
Copy Markdown
Contributor Author

Rebased on master to address conflicts

@findepi
Copy link
Copy Markdown
Member

findepi commented Aug 30, 2023

/test-with-secrets sha=7cb65b2d1451359ce579e76fa22fa5c507e6f0e4

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 30, 2023

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/6028127061

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Aug 30, 2023

Updated mirror PR #18665 within the repository because the /test-with-secrets command failed because of ci.yml change.

@findinpath
Copy link
Copy Markdown
Contributor Author

Unrelated test infra failure:
ci / pt (default, suite-3) failed because not being able to retrieve the docker image from ghcr.io
https://github.com/trinodb/trino/actions/runs/6027449276/job/16353173416?pr=18538

2023-08-30T16:51:12.761Z	ERROR	main	io.trino.tests.product.launcher.cli.TestRun	Failure
org.testcontainers.containers.ContainerFetchException: Can't get Docker image: RemoteDockerImage(imageName=ghcr.io/trinodb/testing/hdp2.6-hive-kerberized:81, imagePullPolicy=io.trino.testing.containers.ConditionalPullPolicy@4978777f, imageNameSubstitutor=org.testcontainers.utility.ImageNameSubstitutor$LogWrappedImageNameSubstitutor@4215e133)

@ebyhr ebyhr merged commit fa5a890 into trinodb:master Aug 31, 2023
@github-actions github-actions bot added this to the 426 milestone Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed delta-lake Delta Lake connector docs no-release-notes This pull request does not require release notes entry test

Development

Successfully merging this pull request may close these issues.

3 participants