Skip to content

Conversation

@martijnvg
Copy link
Member

@martijnvg martijnvg commented Nov 2, 2018

An auto follow pattern name:

  • cannot start with _
  • cannot contain a ,
  • can be encoded in UTF-8
  • the length of UTF-8 encoded bytes is no longer than 255 bytes

Based on the restrictions for ILM policy names: #34928 (comment)

An auto follow pattern:
* cannot start with `_`
* cannot contain a `,`
* can be encoded in UTF-8
* the length of UTF-8 encoded bytes is no longer than 255 bytes
@martijnvg martijnvg added >non-issue :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features labels Nov 2, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@martijnvg martijnvg requested a review from jasontedor November 2, 2018 10:40
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

I left some quibbles about the tests.

assertThat(validationException, nullValue());
}

public void testValidateName() {
Copy link
Member

Choose a reason for hiding this comment

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

Would you break each of the separate test cases into a separate test? testValidateNameComma, testValidateNameLeadingUnderscore, ...

Also, I'm not seeing a test that a name can otherwise contain an underscore as long as it's not the leader character.

@martijnvg
Copy link
Member Author

@jasontedor I've split the tests and also test that underscores are allowed, just not as first character.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@martijnvg martijnvg merged commit 314b9ca into elastic:master Nov 7, 2018
martijnvg added a commit that referenced this pull request Nov 7, 2018
An auto follow pattern:
* cannot start with `_`
* cannot contain a `,`
* can be encoded in UTF-8
* the length of UTF-8 encoded bytes is no longer than 255 bytes
martijnvg added a commit that referenced this pull request Nov 7, 2018
An auto follow pattern:
* cannot start with `_`
* cannot contain a `,`
* can be encoded in UTF-8
* the length of UTF-8 encoded bytes is no longer than 255 bytes
jasontedor added a commit to martijnvg/elasticsearch that referenced this pull request Nov 8, 2018
* master: (24 commits)
  Replicate index settings to followers (elastic#35089)
  Rename RealmConfig.globalSettings() to settings() (elastic#35330)
  [TEST] Cleanup FileUserPasswdStoreTests (elastic#35329)
  Scripting: Add back lookup vars in score script (elastic#34833)
  watcher: Fix integration tests to ensure correct start/stop of Watcher (elastic#35271)
  Remove ALL shard check in CheckShrinkReadyStep (elastic#35346)
  Use soft-deleted docs to resolve strategy for engine operation (elastic#35230)
  [ILM] Check shard and relocation status in AllocationRoutedStep (elastic#35316)
  Ignore date ranges containing 'now' when pre-processing a percolator query (elastic#35160)
  Add a frozen engine implementation (elastic#34357)
  Put a fake allocation id on allocate stale primary command (elastic#34140)
  [CCR] Enforce auto follow pattern name restrictions (elastic#35197)
  [ILM] rolling upgrade tests (elastic#35328)
  [ML] Add Missing data checking class (elastic#35310)
  Apply `ignore_throttled` also to concrete indices (elastic#35335)
  Make version field names more meaningful  (elastic#35334)
  [CCR] Added HLRC support for pause follow API (elastic#35216)
  [Docs] Improve Convert Processor description (elastic#35280)
  [Painless] Removes extraneous compile method (elastic#35323)
  [CCR] Fail with a better error if leader index is red (elastic#35298)
  ...
pgomulka pushed a commit to pgomulka/elasticsearch that referenced this pull request Nov 13, 2018
An auto follow pattern:
* cannot start with `_`
* cannot contain a `,`
* can be encoded in UTF-8
* the length of UTF-8 encoded bytes is no longer than 255 bytes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Indexing/CCR Issues around the Cross Cluster State Replication features >non-issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants