[Backport 2.x] JSON Catalog Reader for Integrations#1392
Merged
Conversation
* Add full statics scan method
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add sample data to serialization method
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Finish integration serialization
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Rename integration reader and add stub json adaptor
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add JsonCatalogDataAdaptor directory type checking
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add integration version filtering to json_data_adaptor
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Remove unviable serialization implementation
The existing serialization implementation was not able to recover all data, as mapping information associating file names with the content was lost in appending the file content at the end of the JSON object.
In order to work around this, a new serialization method needs to add this association.
The most viable solution I see is to add encoded file data right next to the file names within the config, which will make it slightly harder to read if word wrap is enabled, but otherwise solves the issue.
Going to clear this method out entirely for now and restore old functionality with TDD.
Alternative solution: We can make a loose association by making heavy assumptions about array ordering and with complicated parsing logic, but I'm not sure it's worth it.
Better to just go back to the drawing board and make a serialization method that's more flexible.
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add new static serialization
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add component and sample data serialization
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Fix serialization test for new serialization approach
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add new asset serialization
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Fix broken compareVersions comparison and version sorting
When I noticed that adaptors had to handle version sorting, I started writing a test to verify the versions were being sorted.
While verifying the test, I noticed a weird result where 2.0.0 was being sorted after both 1.0.0 and 2.1.0.
After moving the comparison function over to integration_reader where it should be and writing tests,
I saw the method was falsely saying 2.0.0 and 2.1.0 were equal.
Debugging the comparison, I found that the version '1.2.3' was being parsed as [1, NaN, NaN].
Several more minutes of banging my head against my desk, and I realized that the suspect was this line:
`a.split('.').map(Number.parseInt)`.
Because parseInt takes two arguments (number and radix), map was giving it the part *and the index*.
The error went away after fixing the method to use only the part.
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add ability to read config file from json integration
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add rudimentary integration search to JSONAdaptor
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Create readRawConfig method to avoid data pruning
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add localized config parsing to getAssets
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Convert getSampleData to use localized config
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Move localized reader logic to helper method
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Convert getSchemas to use new data retrieval
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add localized config handling for getStatic
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add basic validation test for JSON reader
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add static validation to integration reader
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add more tests for deepCheck on JSON catalog
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add tests for json adaptor edge cases
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add reserialization tests
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add tests and remove redundant error checks for 90% coverage
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Fix lints in repository.test.ts
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Fix lints for integrations_builder.ts
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Update snapshots
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Revert "Update snapshots"
This reverts commit 62b238d.
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Use semver library for comparison
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Switch to upstream NDJson parser
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add inline documentation for IntegrationPart
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Move deepCheck to utils
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add further utility methods to utils
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Refactor repository tests to not rely on result.ok == false
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
---------
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
(cherry picked from commit 6bd872c)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot
bot
requested review from
Swiddis,
YANG-DB,
anirudha,
dai-chen,
derek-ho,
joshuali925,
kavithacm,
mengweieric,
penghuo,
ps48,
rupal-bq,
seankao-az and
vamsimanohar
as code owners
Swiddis
approved these changes
Jan 30, 2024
6 tasks
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## 2.x #1392 +/- ##
==========================================
+ Coverage 53.53% 54.19% +0.66%
==========================================
Files 312 314 +2
Lines 10962 11114 +152
Branches 2879 2921 +42
==========================================
+ Hits 5868 6023 +155
+ Misses 5046 5043 -3
Partials 48 48
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
mengweieric
approved these changes
Jan 31, 2024
amsiglan
pushed a commit
to amsiglan/dashboards-observability
that referenced
this pull request
Jun 7, 2024
…ct#1392) * Add full statics scan method * Add sample data to serialization method * Finish integration serialization * Rename integration reader and add stub json adaptor * Add JsonCatalogDataAdaptor directory type checking * Add integration version filtering to json_data_adaptor * Remove unviable serialization implementation The existing serialization implementation was not able to recover all data, as mapping information associating file names with the content was lost in appending the file content at the end of the JSON object. In order to work around this, a new serialization method needs to add this association. The most viable solution I see is to add encoded file data right next to the file names within the config, which will make it slightly harder to read if word wrap is enabled, but otherwise solves the issue. Going to clear this method out entirely for now and restore old functionality with TDD. Alternative solution: We can make a loose association by making heavy assumptions about array ordering and with complicated parsing logic, but I'm not sure it's worth it. Better to just go back to the drawing board and make a serialization method that's more flexible. * Add new static serialization * Add component and sample data serialization * Fix serialization test for new serialization approach * Add new asset serialization * Fix broken compareVersions comparison and version sorting When I noticed that adaptors had to handle version sorting, I started writing a test to verify the versions were being sorted. While verifying the test, I noticed a weird result where 2.0.0 was being sorted after both 1.0.0 and 2.1.0. After moving the comparison function over to integration_reader where it should be and writing tests, I saw the method was falsely saying 2.0.0 and 2.1.0 were equal. Debugging the comparison, I found that the version '1.2.3' was being parsed as [1, NaN, NaN]. Several more minutes of banging my head against my desk, and I realized that the suspect was this line: `a.split('.').map(Number.parseInt)`. Because parseInt takes two arguments (number and radix), map was giving it the part *and the index*. The error went away after fixing the method to use only the part. * Add ability to read config file from json integration * Add rudimentary integration search to JSONAdaptor * Create readRawConfig method to avoid data pruning * Add localized config parsing to getAssets * Convert getSampleData to use localized config * Move localized reader logic to helper method * Convert getSchemas to use new data retrieval * Add localized config handling for getStatic * Add basic validation test for JSON reader * Add static validation to integration reader * Add more tests for deepCheck on JSON catalog * Add tests for json adaptor edge cases * Add reserialization tests * Add tests and remove redundant error checks for 90% coverage * Fix lints in repository.test.ts * Fix lints for integrations_builder.ts * Update snapshots * Revert "Update snapshots" This reverts commit 62b238d. * Use semver library for comparison * Switch to upstream NDJson parser * Add inline documentation for IntegrationPart * Move deepCheck to utils * Add further utility methods to utils * Refactor repository tests to not rely on result.ok == false --------- (cherry picked from commit 6bd872c) Signed-off-by: Simeon Widdis <sawiddis@amazon.com> 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> (cherry picked from commit 216aa92)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport 6bd872c from #1288.