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

Bug fixes for SQL UI buttons #147

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Conversation

ps48
Copy link
Member

@ps48 ps48 commented Oct 5, 2023

Description

Adding bug fixes for SQL UI buttons

  1. Show create Button only on S3 datasources
  2. Show explain and download buttons only on OpenSearch datasource
  3. Remove text editor disabling
  4. Update index name regex

Issues Resolved

#122

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • 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.

Signed-off-by: Shenoy Pratik <[email protected]>
@ps48
Copy link
Member Author

ps48 commented Oct 5, 2023

Looking into test failure updated snapshots and tests

@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

Merging #147 (dfc281c) into main (56a6bce) will decrease coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #147      +/-   ##
==========================================
- Coverage   54.79%   54.72%   -0.07%     
==========================================
  Files          33       33              
  Lines        1356     1365       +9     
  Branches      210      216       +6     
==========================================
+ Hits          743      747       +4     
- Misses        535      537       +2     
- Partials       78       81       +3     
Flag Coverage Δ
dashboards-query-workbench 54.72% <ø> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 5 files with indirect coverage changes

Signed-off-by: Shenoy Pratik <[email protected]>
@@ -78,7 +78,7 @@ export const ACCELERATION_INDEX_NAME_INFO = `All OpenSearch acceleration indices
- 'Materialized View' indices also enable users to define their index name, but they do not have a suffix.
- An example of a 'Materialized View' index name might look like: \`flint_mydatasource_mydb_mytable_myindexname\`.
##### Note:
- All user given index names must be in lowercase letters. Cannot begin with underscores or hyphens. Spaces, commas, and characters :, ", *, +, /, \, |, ?, #, >, or < are not allowed.
- All user given index names must be in lowercase letters. Index name cannot begin with underscores. Spaces, commas, and characters -, :, ", *, +, /, \, |, ?, #, >, or < are not allowed.
Copy link
Member

Choose a reason for hiding this comment

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

i think i asked this somewhere else but if

All user given index names must be in lowercase letters

and -, :, ", *, +, /, \, |, ?, #, >, or < are obviously not lowercase letters, why does it mention those characters are not allowed?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is taken from here: https://docs.aws.amazon.com/opensearch-service/latest/developerguide/indexing.html
Docs team didn't get time for a review.

@ps48 ps48 merged commit c819cd4 into opensearch-project:main Oct 5, 2023
8 of 9 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 5, 2023
* adding paper cuts

Signed-off-by: Shenoy Pratik <[email protected]>

* update snapshots

Signed-off-by: Shenoy Pratik <[email protected]>

---------

Signed-off-by: Shenoy Pratik <[email protected]>
(cherry picked from commit c819cd4)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 5, 2023
* adding paper cuts

Signed-off-by: Shenoy Pratik <[email protected]>

* update snapshots

Signed-off-by: Shenoy Pratik <[email protected]>

---------

Signed-off-by: Shenoy Pratik <[email protected]>
(cherry picked from commit c819cd4)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ps48 pushed a commit that referenced this pull request Oct 5, 2023
* adding paper cuts



* update snapshots



---------


(cherry picked from commit c819cd4)

Signed-off-by: Shenoy Pratik <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ps48 pushed a commit that referenced this pull request Oct 5, 2023
* adding paper cuts



* update snapshots



---------


(cherry picked from commit c819cd4)

Signed-off-by: Shenoy Pratik <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

3 participants