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

[Transform] enhance the output of preview to return full destination index details #53572

Merged

Conversation

hendrikmuhs
Copy link
Contributor

changes the output format of preview regarding deduced mappings and enhances it to return all the details about auto-index creation. This allows the user to customize the index creation. Using HLRC you can create a index request from the output of the response. Long term this allows us to add UI support, see elastic/kibana#57059 and/or improve setting defaults.

The output of the preview itself is not affected. The UI does not use/display mappings at the moment (CC @walterra )

Breaking:

  • supported:
    • mixed clusters internal communication, all versions
    • HLRC >=7.7 <-> <7.7
  • not supported:
    • HRLC <7.7 <-> >=7.7 (mappings are parsed as empty)

New format:

{ 
    "preview": [
       ...
    ],
    "generated_dest_index": {
        "mappings" : {
        ...
        },
        "settings": {
            "index" : {
                "number_of_shards" : 1,
                "auto_expand_replicas" : "0-1",
                ...
            }
        },
        "aliases": {}
    }
}

Please raise concerns about the naming: generated_dest_index.

Old format:

{ 
    "preview": [
       ...
    ],
    "mappings" : {
       ...
    }
}

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml/Transform)

@hendrikmuhs hendrikmuhs force-pushed the transform-preview-full-dest-details branch from 5dd8013 to b466fb0 Compare March 14, 2020 10:07
@benwtrent benwtrent self-requested a review March 16, 2020 11:38
Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

love this change.

One comment about a logging level, but looks good.

Tricky thing with HLRC, all versions need to be forward-compatible with minor versions in the same major.

This change should be OK as the HLRC continues to work as a whole.

@hendrikmuhs hendrikmuhs merged commit fa6d197 into elastic:master Mar 17, 2020
@hendrikmuhs hendrikmuhs deleted the transform-preview-full-dest-details branch March 17, 2020 20:05
hendrikmuhs pushed a commit to hendrikmuhs/elasticsearch that referenced this pull request Mar 17, 2020
…tic#53572)

changes the output format of preview regarding deduced mappings and enhances
it to return all the details about auto-index creation. This allows the user
to customize the index creation. Using HLRC you can create a index request
from the output of the response.
hendrikmuhs pushed a commit that referenced this pull request Mar 18, 2020
changes the output format of preview regarding deduced mappings and enhances
it to return all the details about auto-index creation. This allows the user
to customize the index creation. Using HLRC you can create a index request
from the output of the response.

backport #53572
@walterra
Copy link

Just caught up with emails and missed the CC unfortunately: We use mappings in the UI to infer field types for the preview tables. I'll come up with a fix this afternoon!

walterra added a commit to elastic/kibana that referenced this pull request Mar 19, 2020
- Fixes regression caused by elastic/elasticsearch#53572.
- Adjusts the TS mappings and code to reflect the newly returned API response.
- Re-enables functional tests.
walterra added a commit to elastic/kibana that referenced this pull request Mar 20, 2020
- Fixes regression caused by elastic/elasticsearch#53572.
- Adjusts the TS mappings and code to reflect the newly returned API response.
- Re-enables functional tests.
@walterra
Copy link

Fixed in Kibana for master/7.x: elastic/kibana#60609

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants