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

Upload API should accepts any valid index name as input #183

Closed
VijayanB opened this issue Oct 28, 2022 · 10 comments
Closed

Upload API should accepts any valid index name as input #183

VijayanB opened this issue Oct 28, 2022 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@VijayanB
Copy link
Member

Upload API accepts requests only when index name ends with -map. This was added because api was initially planned to call only by Region maps. With Maps from 2.5, the upload API should allow user to upload any valid GeoJSON with any acceptable valid index name ( no mandatory suffix ). This won't affect Region maps, since front end already have this validation in place.

@VijayanB VijayanB changed the title Upload API accepts index name that ends with -map Upload API should accepts any valid index name as input Oct 28, 2022
@heemin32
Copy link
Collaborator

This won't affect Region maps, since front end already have this validation in place.

Is that mean, we need such restriction for Region map? We cannot rely on frontend validation. Customer request can hit the backend directly.

@navneet1v
Copy link
Collaborator

navneet1v commented Oct 29, 2022

@VijayanB The description poses 2 questions:

  1. For region maps do need maps as suffix on the index name?
  2. With 2.5 release of dashboard are we thinking that a valid geojson can be applied apart from region maps?

+1 on @heemin32 comment, we need to have validations at both the places but before we see where we need to put validation lets discuss on above 2 questions first.

@VijayanB
Copy link
Member Author

VijayanB commented Oct 29, 2022

For question 2, yes, you can use it in cluster and document layers.
For question 1, it was added to easily identify index that was created by file upload.

In fact, this will make backend API generic and decoupled from Dashboards. We are extending the existing API to support more generic cases.

@navneet1v
Copy link
Collaborator

@VijayanB I think it make sense, but as region maps older UI will also be supported in 2.5 release the check on the OSD will still remain. We need to make sure that we are removing that check also in this case.

@heemin32
Copy link
Collaborator

Is there other ways to identify index that was created by file upload without using the suffix now?

@VijayanB
Copy link
Member Author

VijayanB commented Oct 30, 2022

Sorry it was not clear from description. Let me try to explain how it is handled now. From OpenSearch Dashboards 2.3 onwards, region map will call this API by adding suffix "-map" as index name. For ex: if user gives test-index, then OSD sends index name as test-index-map as input name. In the backend, this input is validated for suffix. This was added as extra validation to make sure that this API is used only for Region Maps. This "-map" suffix is required so that region map can ask list of index that ends with "-map" and suggest only those index for custom vector map.

Now, we can leverage the same API to create an index for non-Region map use cases . Hence, i am proposing to remove this check only from backend. We still have to add "-map" suffix from OSD for Region map for older UI in-order to be available for Custom Vector Map.

What changed now with this change?
It was expected ( only supported ) that users will use OSD UI to upload Custom Vector Map .
Before this Change:
If they tried to do this from curl or non-dashboard, it will be rejected unless api includes expected suffix.
After this change:
The request will be succeeded, since, this request can be used to create an index that can be used independent of Region Map. If users would like to use this for Region map, they can use it like before.
Here, we are successfully decoupling backend API from OSD requirements.

IMO, i don't think this breaks any backward compatibility.
@navneet1v @heemin32 Please let me know wdyt and do you have any other suggestions?

@heemin32
Copy link
Collaborator

Thanks @VijayanB. It is clear for me now. Agree that we can remove the validation in backend side.

One more question. In the new release, I assume we have another way to get a list of Custom Vector Map without relying on the "-map" suffix?

@VijayanB
Copy link
Member Author

VijayanB commented Oct 30, 2022

@heemin32 We might be taking a different direction where going forward users can use any index with geospatial field types as custom vector map ( it is not finalized ) . We can also explore or revisit some other options on how to identify uploaded indices and only provide those as input for custom vector map. I believe this is scheduled for 2.6 release

@heemin32
Copy link
Collaborator

The needs to remove the validation from backend side is clear as it needs to be used by cases other than Custom Vector Map.

We should have a better way to track a group of indices in OSD without relying on suffix. @VijayanB, could you create an issue in dashboards-maps repo?

@navneet1v navneet1v added the enhancement New feature or request label Nov 30, 2022
@VijayanB VijayanB self-assigned this Jan 17, 2023
@VijayanB
Copy link
Member Author

Feature is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants