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

Update sample data naming conventions #24

Merged
merged 2 commits into from
May 13, 2021

Conversation

ohltyler
Copy link
Member

@ohltyler ohltyler commented May 12, 2021

Description

Current sample detectors and indices include the legacy opendistro- prefix. This PR removes that prefix.

Example:
eCommerce detector change: opendistro-sample-ecommerce-detector => sample-ecommerce-detector
eCommerce index change: opendistro-sample-ecommerce => sample-ecommerce

In addition:

  • Handled backwards compatibility by ensuring legacy detectors created are still recognized as created on the sample detectors page
  • Changed the sample details flyout to pull the detector & index names from an existing detector if possible. That way, new and legacy detectors will both be properly reflected
  • Added legacy checks to helper fns and query parameters to search for both new and legacy created detectors and indices
  • Confirmed newly named detectors/indices function the same as before

Screenshots:

Legacy + new sample detectors coexisting:
Screen Shot 2021-05-12 at 1 24 19 PM
Screen Shot 2021-05-12 at 1 24 35 PM

Details flyout (legacy):
Screen Shot 2021-05-12 at 1 25 02 PM

Details flyout (new):
Screen Shot 2021-05-12 at 1 25 16 PM

Issues Resolved

Resolves #3

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

ohltyler added 2 commits May 12, 2021 11:17
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
@ohltyler ohltyler added the enhancement New feature or request label May 12, 2021
@@ -73,23 +73,23 @@ export default class SampleDataService {
__dirname,
'../sampleData/rawData/httpResponses.json.gz'
);
indexName = 'opendistro-sample-http-responses';
indexName = 'sample-http-responses';
Copy link
Contributor

Choose a reason for hiding this comment

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

If user already has index with name sample-http-responses, will we create sample detector on user's existing index or will throw exception?

Copy link
Member Author

Choose a reason for hiding this comment

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

We will try to create on the existing index, but if it's some user's created index it will most likely fail since it won't have the same fields & mappings, etc.

Basically we just check for the existence of the sample index name here and create if it doesn't already exist.

This check is added to handle the case of user creating the sample data (both detector and index), and maybe deleting or changing the detector name. In that case, they can recreate the sample detector, and will use the existing sample index if it is still there.

This is admittedly simple; one of the tracked improvements for sample detectors is adding delete buttons on the UI and better handling of edge cases

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. Should pay attention when delete sample detectors to avoid delete some user's data index with the same name of sample-http-responses

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Will have to be careful when introducing deletions here. Same goes for detectors.

Copy link
Contributor

@ylwu-amzn ylwu-amzn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the change!

@ohltyler ohltyler merged commit 0d4dd6f into opensearch-project:main May 13, 2021
@ohltyler ohltyler mentioned this pull request Jul 21, 2021
1 task
@ohltyler ohltyler deleted the sample-rename branch July 28, 2021 22:33
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

Successfully merging this pull request may close these issues.

Update sample detectors names & indices
3 participants