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

Add native code info to ML info api #43172

Merged
merged 5 commits into from Jun 13, 2019

Conversation

Projects
None yet
3 participants
@rjernst
Copy link
Member

commented Jun 12, 2019

The machine learning feature of xpack has native binaries with a
different commit id than the rest of code. It is currently exposed in
the xpack info api. This commit adds that commit information to the ML
info api, so that it may be removed from the info api.

Add native code info to ML info api
The machine learning feature of xpack has native binaries with a
different commit id than the rest of code. It is currently exposed in
the xpack info api. This commit adds that commit information to the ML
info api, so that it may be removed from the info api.
@elasticmachine

This comment has been minimized.

Copy link

commented Jun 12, 2019

rjernst added some commits Jun 12, 2019


try {
NativeController nativeController = NativeControllerHolder.getNativeController(clusterService.getNodeName(), env);
assert nativeController != null;

This comment has been minimized.

Copy link
@droberts195

droberts195 Jun 13, 2019

Contributor

This is complicated because in internal cluster tests there is no native controller.

To get the internal cluster tests to work I think you'll have to do something like:

    if (nativeController != null) {
        nativeCodeInfo = nativeController.getNativeCodeInfo();
    } else {
        nativeCodeInfo = Collections.emptyMap();
    }

or if you want to be more rigorous:

    if (MachineLearningField.AUTODETECT_PROCESS.get(env.settings())) {
        NativeController nativeController = NativeControllerHolder.getNativeController(clusterService.getNodeName(), env);
        assert nativeController != null;
        nativeCodeInfo = nativeController.getNativeCodeInfo();
    } else {
        nativeCodeInfo = Collections.emptyMap();
    }

It makes me realise that moving this out of the feature set and into a normal transport action makes it possible to just have the native controller as a standard plugin component instead of having that horrible and complicated singleton holder. I'll refactor that in a followup.

This comment has been minimized.

Copy link
@rjernst

rjernst Jun 13, 2019

Author Member

I went with your first suggestion for now (see f740ccf), and added a comment to remember to do the conversion from the holder once we've removed native code from xpack info.

@droberts195
Copy link
Contributor

left a comment

LGTM

@rjernst rjernst merged commit a3f2f40 into elastic:master Jun 13, 2019

9 checks passed

CLA All commits in pull request signed
Details
elasticsearch-ci/1 Build finished.
Details
elasticsearch-ci/2 Build finished.
Details
elasticsearch-ci/bwc Build finished.
Details
elasticsearch-ci/default-distro Build finished.
Details
elasticsearch-ci/docbldesx Build finished.
Details
elasticsearch-ci/docs-check Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details

@rjernst rjernst deleted the rjernst:deguice43 branch Jun 13, 2019

rjernst added a commit that referenced this pull request Jun 13, 2019

Add native code info to ML info api (#43172)
The machine learning feature of xpack has native binaries with a
different commit id than the rest of code. It is currently exposed in
the xpack info api. This commit adds that commit information to the ML
info api, so that it may be removed from the info api.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.