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

Move schema files to centralized catalog #774

Closed
wants to merge 7 commits into from

Conversation

Swiddis
Copy link
Collaborator

@Swiddis Swiddis commented Jul 28, 2023

Description

Per side-channel discussion, we realized that it's going to block integration development if we need to keep our schemas in sync with each other everywhere. This PR centralizes the catalog files so that variations in schemas can be better tracked. This is a stand-in solution until the catalog API is available at opensearch-catalog.

Following this PR, the following process for updating catalog files is advised:

  1. Create a new copy of the catalog file with an updated version number in accordance with SemVer.
  2. Please sync updates to catalog files back with opensearch-catalog.
  3. Optional: inform authors of integrations using old catalog files that a new one is available. We'd like to release with all integrations using the same catalog files, and prune any redundant copies as we go. We'll need to do one final pass-through before release.

Issues Resolved

N/A

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: Simeon Widdis <sawiddis@amazon.com>
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #774 (2aecf7c) into main (3ca83f8) will decrease coverage by 0.01%.
Report is 3 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #774      +/-   ##
==========================================
- Coverage   43.66%   43.65%   -0.01%     
==========================================
  Files         312      312              
  Lines       18587    18587              
  Branches     4479     4480       +1     
==========================================
- Hits         8116     8115       -1     
- Misses      10429    10430       +1     
  Partials       42       42              
Flag Coverage Δ
dashboards-observability 43.65% <ø> (-0.01%) ⬇️

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

see 2 files with indirect coverage changes

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis Swiddis requested a review from YANG-DB July 28, 2023 23:28
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@YANG-DB YANG-DB added the integrations Used to denote items related to the Integrations project label Jul 31, 2023
@@ -4,7 +4,7 @@
"displayName": "AWS VPC Flow",
"description": "AWS VPC Flow log collector",
"license": "Apache-2.0",
"type": "logs",
"type": "logs_vpc",
Copy link
Member

Choose a reason for hiding this comment

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

all the logs index based integrations should use the same index template names logs
the only dynamic things that should be integration specific are:

  • index alias
  • composed_of component list

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We discussed that we need to work with these duplicates for now to account for multiple component configs. Let's work on fixing it later, as a hacky solution I can try setting the composed_of list dynamically, but I want to keep this PR small

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Copy link
Collaborator Author

Swiddis commented Aug 1, 2023

Todo: Update front-end display to trim leading directories from categories.

Actual: If you open the VPC integration you'll see the categories listed as aws/vpc_flow, cloud, communication, logs_vpc, aws/s3.

Expected: Trim leading directories for vpc_flow, cloud, communication, logs_vpc, s3, and possibly capitalize everything + remove underscores to make it look nicer? Still a little awkward but easy to implement and a step in the right direction. Vpc Flow, Cloud, Communication, Logs Vpc, S3.

@Swiddis Swiddis marked this pull request as draft August 4, 2023 19:29
@Swiddis
Copy link
Collaborator Author

Swiddis commented Aug 9, 2023

Status update: Postponing to after we're done with 2.9 release. This is a hefty refactor and will need some time to get done right. Will leave it as a draft for now, we can revisit it in a month or so.

@YANG-DB
Copy link
Member

YANG-DB commented Aug 10, 2023

resolved in opensearch-project/opensearch-catalog#69

@Swiddis Swiddis closed this Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Used to denote items related to the Integrations project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants