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.

Sign up for GitHub

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

Jump to bottom

ADDON-34273: globalConfig.json parsing and validation #111

Merged
merged 6 commits into from
Mar 3, 2021
Merged

ADDON-34273: globalConfig.json parsing and validation #111

merged 6 commits into from
Mar 3, 2021

Conversation

tbalar-splunk
Copy link
Contributor

JIRA Ticket: https://jira.splunk.com/browse/ADDON-34273

  • Validating the globalConfig.json file
  • Showing error message if file not found
  • Showing error message if validation is failed

@github-actions
Copy link

github-actions bot commented Mar 3, 2021

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

I have read the CLA Document and I hereby sign the CLA

You can retrigger this bot by commenting recheck in this Pull Request

mamin-crest
mamin-crest reviewed Mar 3, 2021
View reviewed changes
Copy link
Contributor

@mamin-crest mamin-crest left a comment

Choose a reason for hiding this comment

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

I think, We should also fix eslint errors(which we can fix now. E.g. declaration of props-types, etc) along with development. So, it won't take extra efforts later.
@harshpatel-crest @tbalar-splunk @mchavda-splunk @dkhatri-crest
What do you think??

tbalar-splunk
tbalar-splunk commented Mar 3, 2021
View reviewed changes
Copy link
Contributor Author

@tbalar-splunk tbalar-splunk left a comment

Choose a reason for hiding this comment

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

@mamin-crest Will add the propTypes in the next PR

harshpatel-crest
harshpatel-crest approved these changes Mar 3, 2021
View reviewed changes
splunk_add_on_ucc_framework/ucc_ui_lib/src/main/webapp/util/configManager.js
<>
{
this.state.loading ?
<WaitSpinner size="large" /> :
Copy link
Contributor

Choose a reason for hiding this comment

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

can we center the WaitSpinner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I Will add in the next PR

splunk_add_on_ucc_framework/ucc_ui_lib/src/main/webapp/util/uccConfigurationValidators.js
@@ -0,0 +1,255 @@
import schema from '../schema/schema.json';
import {Validator} from 'jsonschema';
import * as _ from 'lodash';
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible we should avoid such large dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will remove it after discussing it in the scrum

splunk_add_on_ucc_framework/ucc_ui_lib/src/main/webapp/util/messageUtil.js
import messageDict from '../constants/messageDict';

/**
* @param code {Int} a int value.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should follow this docstring pattern everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this PR most of the javascript code is taken from the swc. So next time will add the docstring pattern in the function if necessary

mamin-crest
mamin-crest approved these changes Mar 3, 2021
View reviewed changes
splunk_add_on_ucc_framework/ucc_ui_lib/src/main/webapp/util/util.js Outdated
}

export function generateEndPointUrl(name) {
console.log("unifiedConfig: ", unifiedConfigs);
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we put this console.log intentionally ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, It's just for reference to how to get unifiedConfigs data

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure because, you already have put example in TestComponent.jsx. and this is util file. So this will log everytime we call generateEndPointUrl.

splunk_add_on_ucc_framework/ucc_ui_lib/src/main/webapp/schema/schema.json
@@ -0,0 +1,1254 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, We should also add schemaGenerator.py along with schema.json as schema.json generated by it. So, If we want to change schema.json. We'll need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@github-actions
Copy link

github-actions bot commented Mar 3, 2021

Expected release notes (by @tbalar-splunk)

features:
Removed log line (1f51cb4)
Added schemaGenerator.py file (be0b945)
Fixed issue (887f419)
Fixed typo (668d2a5)
Added error modal component (bbde4cf)
Added parsing and validation functionality for globalConfig.json file (e690000)

fixes:
title in input and configuration page templates (6328fdf)
commented oauth realated logic from ucc-gen temporarily (6d08509)

others (will not be included in Semantic-Release notes):
ucc_ui_lib: updated webpack config according to guideline mentioned in react doc for code-splitting (57a81ec)
ucc_ui_lib: removed unused import (e490b81)
ucc_ui_lib: updated configs and structure to have single entry point js and template file (e04d1db)
d234bfb fix the output path for static files
update build scripts and circle ci config (a949f40)
ef5c95a removed old ucc ui lib
migrate to splunk-ui template structure and ci (bd099f4)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

@tbalar-splunk tbalar-splunk merged commit 24679f3 into ucc_ui_lib_migration Mar 3, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2021
@ryanfaircloth
Copy link
Contributor

🎉 This PR is included in version 4.4.0-develop.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@ryanfaircloth
Copy link
Contributor

🎉 This PR is included in version 5.0.0-develop.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@ryanfaircloth
Copy link
Contributor

🎉 This PR is included in version 5.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@ghost ghost deleted the feature/ADDON-34273-global-config-parsing-and-validation branch July 12, 2021 09:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mamin-crest mamin-crest mamin-crest approved these changes

@mchavda-splunk mchavda-splunk mchavda-splunk approved these changes

@harshpatel-crest harshpatel-crest harshpatel-crest approved these changes

Assignees
No one assigned
Labels
released on @beta released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tbalar-splunk @ryanfaircloth @mchavda-splunk @mamin-crest @harshpatel-crest