Skip to content

Conversation

@romseygeek
Copy link
Contributor

The actual usage of types within the graph code was removed by #42112,
so this commit just removes them from the Rest API

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@jpountz jpountz mentioned this pull request Sep 20, 2019
66 tasks
indices = in.readStringArray();
indicesOptions = IndicesOptions.readIndicesOptions(in);
types = in.readStringArray();
if (in.getVersion().before(Version.V_8_0_0)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I mentioned this in the PR for the explain API as well -- I think it'd be nice to read the types array and assert that it is empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

++

return this;
}

private static final String[] EMPTY_ARRAY = new String[]{};
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of defining this variable, I think we could use Strings.EMPTY_ARRAY below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I knew we had a predefined array somewhere, and for some reason was unable to find it. Thanks!

controller.registerWithDeprecatedHandler(
POST, "/{index}/{type}/_graph/explore", this,
POST, "/{index}/{type}/_xpack/graph/_explore", deprecationLogger);
controller.registerHandler(GET, "/{index}/_graph/explore", this);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we removed the deprecated _xpack endpoints here instead of the ones containing {type}. We could remove the ones with {type} in this PR, and leave the _xpack removal for a future PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, good catch - will fix.

@romseygeek
Copy link
Contributor Author

I also spotted that I hadn't removed types from the Rest-High-Level client; I have updated the PR accordingly.

@romseygeek romseygeek merged commit 8927735 into elastic:master Sep 23, 2019
@romseygeek romseygeek deleted the types-removal/graph-action branch September 23, 2019 16:57
@pgomulka pgomulka mentioned this pull request Mar 25, 2020
66 tasks
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Jun 16, 2021
adds back the typed and xpack endpoints for graph explore api prevoiusly removed in elastic#46935 relates main meta issue elastic#51816 relates types removal issue elastic#54160
pgomulka added a commit that referenced this pull request Jun 23, 2021
adds back the typed and xpack endpoints for graph explore api prevoiusly removed in #46935 relates main meta issue #51816 relates types removal issue #54160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment