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.

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

Merge three Prometheus integration tests into one #12458

Merged
merged 2 commits into from
May 23, 2022

Conversation

zhaoyim
Copy link
Contributor

@zhaoyim zhaoyim commented May 18, 2022

Description

Is this change a fix, improvement, new feature, refactoring, or other?
maintenance, test

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)
No

How would you describe this change to a non-technical end user or system administrator?
Merge three Prometheus integration tests into one

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.

Release notes

(x) No release notes entries required.

@cla-bot cla-bot bot added the cla-signed label May 18, 2022
@hashhar hashhar requested a review from ebyhr May 18, 2022 11:41
@zhaoyim zhaoyim requested a review from findinpath May 18, 2022 16:53
Copy link
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

  • testRetrieveUpValue (handled in PrometheusServer)
  • testMetadata (testDescribeTable is enough)
  • testGetTableHandle (unhelpful)
  • testGetColumnHandles (testDescribeTable is enough)
  • testGetTableMetadata (testDescribeTable is enough)
  • testListTables (testShowTables is enough)

@ebyhr ebyhr added the no-release-notes This pull request does not require release notes entry label May 19, 2022
@zhaoyim
Copy link
Contributor Author

zhaoyim commented May 19, 2022

Please remove

  • testRetrieveUpValue (handled in PrometheusServer)
  • testMetadata (testDescribeTable is enough)
  • testGetTableHandle (unhelpful)
  • testGetColumnHandles (testDescribeTable is enough)
  • testGetTableMetadata (testDescribeTable is enough)
  • testListTables (testShowTables is enough)

Removed

@zhaoyim zhaoyim requested a review from ebyhr May 19, 2022 02:18
@zhaoyim
Copy link
Contributor Author

zhaoyim commented May 19, 2022

Also Squash the commits

@zhaoyim zhaoyim force-pushed the i12492_2 branch 3 times, most recently from cd7d734 to 115b766 Compare May 19, 2022 05:08
Copy link
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 check CI failure.

@zhaoyim zhaoyim force-pushed the i12492_2 branch 3 times, most recently from 547ce24 to 9163821 Compare May 20, 2022 11:09
@ebyhr ebyhr merged commit 5d0b73a into trinodb:master May 23, 2022
@ebyhr
Copy link
Member

ebyhr commented May 23, 2022

Merged, thanks!

@github-actions github-actions bot added this to the 382 milestone May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed no-release-notes This pull request does not require release notes entry
Development

Successfully merging this pull request may close these issues.

Merge three Prometheus integration tests into one
2 participants