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 integration format for better handling of multiple asset types #1498

Merged
merged 11 commits into from
Mar 8, 2024

Conversation

Swiddis
Copy link
Collaborator

@Swiddis Swiddis commented Mar 7, 2024

Description

To support multiple installation paths for integrations better, part of that is refactoring the current install flow to make the different types of assets more uniform. This PR:

Next steps are refactoring the installation flow to support multiple install paths, using these extensability updates.

Notice: I noticed while writing this that we don't have any unit/integration tests that ensure that all types of assets are being correctly installed. This will be included as part of the installation workflows PR, the current PR is already fairly big as-is. Followed the boy scout rule for some other misc. changes like removing dead code and fixing misused types.

Issues Resolved

Closes #1444

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>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis Swiddis changed the title Update integration format to keep Update integration format for better handling of multiple asset types Mar 7, 2024
@Swiddis Swiddis added enhancement New feature or request integrations Used to denote items related to the Integrations project labels Mar 7, 2024
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Copy link

codecov bot commented Mar 7, 2024

Codecov Report

Attention: Patch coverage is 81.69014% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 54.36%. Comparing base (290415a) to head (fc22b95).

Files Patch % Lines
...egrations/components/create_integration_helpers.ts 54.54% 5 Missing ⚠️
...grations/components/__tests__/testing_constants.ts 42.85% 4 Missing ⚠️
...ents/integrations/components/setup_integration.tsx 50.00% 2 Missing ⚠️
...tors/integrations/repository/integration_reader.ts 93.54% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1498      +/-   ##
==========================================
+ Coverage   54.26%   54.36%   +0.10%     
==========================================
  Files         343      344       +1     
  Lines       12338    12353      +15     
  Branches     3173     3173              
==========================================
+ Hits         6695     6716      +21     
+ Misses       5587     5581       -6     
  Partials       56       56              
Flag Coverage Δ
dashboards-observability 54.36% <81.69%> (+0.10%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

name: string;
version: string;
extension: string;
type: SupportedAssetType;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the core change, most of the refactoring is following the consequences of this type update

@@ -190,6 +191,9 @@ export class ObservabilityPlugin
},
},
},
migrations: {
'3.0.0': migrateV1IntegrationToV2Integration,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not 100% sure I've set up migration correctly, messed with it a bit to get the type checking to work out but I'm not sure when this migration triggers.

Copy link
Member

Choose a reason for hiding this comment

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

Hi
How would migration be handled an by whom ?
For example if I have a migration script - where can I specify it ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Migrations run on OSD startup, it's handled in OSD and will apply to all of the installed templates that are on a lower version. But I'm not sure how OSD detects the version.

For content, I migrated all of the prebuilt content, but I'll need to update the bundle script for the new version.

@Swiddis Swiddis merged commit b5ee6f8 into opensearch-project:main Mar 8, 2024
20 of 25 checks passed
@Swiddis Swiddis deleted the feature/conditional-install branch March 8, 2024 18:12
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 8, 2024
* Update integration type for new format

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* Update integration type validator for new type

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* Fix type errors in integration_reader

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* Fix return types in adaptor

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* Update integrations constants

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* Fix local repository and local repository tests

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* Fix remaining integrations unit tests

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* Add integration format migration

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* Fix frontend usage of integration assets

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* Update integration install query iteration

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

---------

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
(cherry picked from commit b5ee6f8)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
RyanL1997 pushed a commit to RyanL1997/dashboards-observability that referenced this pull request Apr 18, 2024
)

* Fix a bad import path

- Resolves opensearch-project#1487

Signed-off-by: Peter Nied <petern@amazon.com>
Signed-off-by: Peter Nied <peternied@hotmail.com>
Co-authored-by: Ryan Liang <109499885+RyanL1997@users.noreply.github.com>
(cherry picked from commit 32c02c8c18a509884613517fac38b684338d5273)

Co-authored-by: Peter Nied <petern@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x enhancement New feature or request integrations Used to denote items related to the Integrations project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Refactor integration setup process
3 participants