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

Backport of IndexServiceTest #10628

Merged
merged 2 commits into from Oct 13, 2020
Merged

Backport of IndexServiceTest #10628

merged 2 commits into from Oct 13, 2020

Conversation

mkleen
Copy link
Contributor

@mkleen mkleen commented Oct 7, 2020 •
edited

Summary of the changes / Why this improves CrateDB

Backport of IndexServiceTest and depending elastic/elasticsearch@3c352a8

See individual commits.

Checklist

  • Added an entry in CHANGES.txt for user facing changes
  • Updated documentation & sql_features table for user facing changes
  • Touched code is covered by tests
  • CLA is signed
  • This does not contain breaking changes, or if it does:
    • It is released within a major release
    • It is recorded in CHANGES.txt
    • It was marked as deprecated in an earlier release if possible
    • You've thought about the consequences and other components are adapted
      (E.g. AdminUI)

@mkleen mkleen mentioned this pull request Oct 7, 2020
Closed
14 tasks
@mkleen mkleen marked this pull request as ready for review October 8, 2020 09:55
@mkleen mkleen self-assigned this Oct 8, 2020
@mkleen mkleen force-pushed the mkleen/indexservice_test branch 6 times, most recently from 8b28695 to af97277 Compare October 9, 2020 10:25
mkleen
mkleen commented Oct 9, 2020
View reviewed changes
server/src/test/java/io/crate/integrationtests/TableSettingsTest.java
@@ -88,8 +88,8 @@ public void testSelectSettingsColumn() throws Exception {

@Test
public void testSetNonDynamicTableSetting() {
assertThrows(() -> execute("alter table settings_table set (\"translog.sync_interval\"='10s')"),
isSQLError(containsString("Can't update non dynamic settings [[index.translog.sync_interval]] for open indices"),
assertThrows(() -> execute("alter table settings_table set (\"soft_deletes.enabled\"='true')"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since translog.sync_interval is dynamic now we need to test against a different non dynamic setting.

@mkleen mkleen force-pushed the mkleen/indexservice_test branch 3 times, most recently from 55e6dea to 3f4b73e Compare October 9, 2020 12:55
@mkleen mkleen marked this pull request as draft October 12, 2020 08:22
@mkleen mkleen force-pushed the mkleen/indexservice_test branch 4 times, most recently from 8f40c99 to 13ca785 Compare October 12, 2020 15:07
@mkleen mkleen marked this pull request as ready for review October 12, 2020 15:12
@mkleen
Copy link
Contributor Author

mkleen commented Oct 12, 2020

Ready for review.

@mkleen mkleen force-pushed the mkleen/indexservice_test branch 2 times, most recently from 68625c7 to 5837361 Compare October 13, 2020 11:53
@mkleen
Copy link
Contributor Author

mkleen commented Oct 13, 2020

retest this please

@mkleen
Copy link
Contributor Author

mkleen commented Oct 13, 2020

The squashing went wrong, one moment.

@mkleen
Copy link
Contributor Author

mkleen commented Oct 13, 2020

retest this please

@mkleen
Copy link
Contributor Author

mkleen commented Oct 13, 2020

Ok, ready for review. Last flaky test is also fixed.

@mkleen mkleen force-pushed the mkleen/indexservice_test branch 2 times, most recently from 4bace59 to fd7cd25 Compare October 13, 2020 15:22
@mkleen mkleen added the ready-to-merge Let Mergify merge the PR once approved and checks pass label Oct 13, 2020
@mergify mergify bot merged commit a3604df into master Oct 13, 2020
@mergify mergify bot deleted the mkleen/indexservice_test branch October 13, 2020 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfussenegger mfussenegger mfussenegger approved these changes

Assignees

@mkleen mkleen

Labels
ready-to-merge Let Mergify merge the PR once approved and checks pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mkleen @mfussenegger