Add JSON5 parsing capabilities for integration configs

[devesh-2002]

Description

Integrated JSON5

Issues Resolved

Issue #1680

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.

[RyanL1997]

Hi @devesh-2002, thank you for taking this on! In generally this is looking good to me, however, could you also sign off for your commit?

git commit -s -m ""

[devesh-2002]

Is this okay?

[Swiddis]

From the DCO action details, signing off has 3 steps:

1. Ensure you have a local copy of your branch by checking out the pull request locally via command line.
2. In your local branch, run: git rebase HEAD~4 --signoff
3. Force push your changes to overwrite the branch: git push --force-with-lease origin devesh1

As far as the PR goes: This looks good at a glance, thanks for contributing! You might also consider adding some unit tests (or modifying existing ones) to cover the new functionality. I'll be available later this week to get this merged/backported.

[devesh-2002]

Hello. I have signed off the commits and I have also tried to add some tests. Could you please check it? @Swiddis

[Swiddis]

Thanks for adding DCO and tests. I enabled the CI workflows for the PR to help narrow the feedback loop, but I think there's still some more changes to look at. Thanks again for contributing!

[devesh-2002]

Hello,@Swiddis. Can you check if it is correct now?

[Swiddis]

You can run tests locally with yarn test, you don't need to wait for the CI here to get approved (we can't approve the CI here if there's merge conflicts). Running yarn test integrations:

$ yarn test integrations
...
Test Suites: 4 failed, 19 passed, 23 total
Tests:       18 failed, 136 passed, 154 total
Snapshots:   15 passed, 15 total
Time:        81.812 s

In particular the tests in integrations/__test__/local_fs_repository.test.ts will be more informative since they all have to do with the local files you're modifying (and the other tests depend on them passing):

$ yarn test integrations/__test__/local_fs_repository
Test Suites: 1 failed, 1 total
Tests:       3 failed, 1 passed, 4 total
Snapshots:   0 total
Time:        4.588 s, estimated 72 s

In this case it's because the file extension was changed to json5, but there's no handling for json5 filenames. You can try to implement this handling in the code if you'd like, or you can change the extensions back to json but keep the json5-formatted data here.

[devesh-2002]

I have changed the extensions to back to JSON keeping the data as JSON5.

[Swiddis]

Looks like it's mostly working now, just some remaining unit tests that need to be updated for the new error messages: https://github.com/opensearch-project/dashboards-observability/actions/runs/8899395226/job/24438654843?pr=1732#step:6:10907

Again, I recommend running tests locally instead of waiting on actions here, it makes for a faster feedback loop and the tests are typically good at catching these types of small compatibility issues.

[devesh-2002]

I tried to change the JSON5 to JSON again in json_data_adaptor.ts and when I do yarn test integrations the tests are running correctly

Test Suites: 23 passed, 23 total
Tests:       154 passed, 154 total
Snapshots:   15 passed, 15 total
Time:        1424.374 s
Ran all test suites matching /integrations/i.

[Swiddis]

Confirmed that tests are passing after fixing upstream breakage -- will merge this after #1792. Thanks again for the contribution!

[devesh-2002]

Thanks a lot for all the suggestions in this PR! @Swiddis @RyanL1997