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

Vscode extension #1404

Merged
merged 18 commits into from
Oct 3, 2022
Merged

Vscode extension #1404

merged 18 commits into from
Oct 3, 2022

Conversation

eravatee
Copy link
Contributor

@eravatee eravatee commented Jul 12, 2022

The primary body of work to setup a vscode extension for caliper.
It introduces a json schema to assist users in creating a runtime configuration for caliper

To run the extension:

  1. Compile the code by using : npm run compile
  2. Execute the extension from VS code by running the file: extension.js

@eravatee eravatee closed this Jul 14, 2022
@eravatee eravatee reopened this Jul 14, 2022
@eravatee eravatee force-pushed the vscode-extension branch 2 times, most recently from c4de12d to e662c83 Compare July 14, 2022 20:14
Copy link
Contributor

@aklenik aklenik left a comment

Choose a reason for hiding this comment

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

Besides the comments embedded in the files, I have the following general remarks:

  1. The package directory name should be caliper-vscode-extension
  2. This directory should be the package's root, so no other extension subdirectory is needed.
  3. The schema files should go into the caliper-vscode-extension/artifacts/schemas directory. We might have different types of artifacts later on.
  4. The schema file names should be more descriptive (their length doesn't matter, users will rarely type it themselves):
    1. benchmark.json -> hyperledger-caliper-benchmark-confguration-schema.json
    2. default.json -> hyperledger-caliper-runtime-configuration-schema.json
    3. fabric.json -> hyperledger-caliper-fabric-network-configuration-schema.json
    4. ethereum-besu.json -> hyperledger-caliper-ethereum-network-configuration-schema.json
  5. You should check whether the current configuration files used by the CI pipeline (in packages/caliper-tests-integration) pass the schema validation. Those configs cover a lot of different use cases, so until we don't have proper unit tests for the schemas, such smoke tests can help.
  6. Since the extension only registers schemas, we won't have any TS source files in the PR for now. So every unrelated sources can be removed from the PR.
  7. For starters, I suggest keeping only the runtime configuration schema for this PR. The other schemas are more complicated, and even the runtime schema is incomplete regarding descriptions.

packages/caliper-vscode/extension/package.json Outdated Show resolved Hide resolved
packages/caliper-vscode/extension/package.json Outdated Show resolved Hide resolved
{
"name": "caliper-vscode-extension",
"displayName": "caliper-vscode-extension",
"description": "",
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be filled with a single, short, descriptive sentence about the extension. Look up some extensions in vscode to get a sense of what kinds of descriptions should be used.

packages/caliper-vscode/extension/package.json Outdated Show resolved Hide resolved
packages/caliper-vscode/extension/package.json Outdated Show resolved Hide resolved
packages/caliper-vscode/extension/package.json Outdated Show resolved Hide resolved
@@ -0,0 +1,17 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

This should extend the available Node14 base:

packages/caliper-vscode/extension/CHANGELOG.md Outdated Show resolved Hide resolved
@@ -0,0 +1,24 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

You should reuse the eslintrc file content from one of the other packages, like caliper-core, so it's in sync with the other packages

"description": "The number of TXs Caliper should submit during the round.",
"type": "integer"
},
"rateControl": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on the value of type, the opts object can be different. I'd suggest exploring conditional schemas to handle such cases flexibly: https://json-schema.org/understanding-json-schema/reference/conditionals.html#if-then-else
This will require a newer JSON schema version, but vscode supports even the newest draft, so I'd suggest that every schema should use that newest draft.

@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

❗ No coverage uploaded for pull request base (main@8a792e0). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1404   +/-   ##
=======================================
  Coverage        ?   55.56%           
=======================================
  Files           ?      103           
  Lines           ?     4451           
  Branches        ?      676           
=======================================
  Hits            ?     2473           
  Misses          ?     1435           
  Partials        ?      543           
Flag Coverage Δ
caliper-core 44.31% <0.00%> (?)
caliper-fabric 80.46% <0.00%> (?)
generator-caliper 83.62% <0.00%> (?)

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

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@aklenik aklenik left a comment

Choose a reason for hiding this comment

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

There are still some unresolved remarks or solutions that don't follow the suggested approaches (eslint, tsconfig contents, left-over out-of-scope schemas). Please resolve these systematically.

@@ -0,0 +1,350 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

For now, let's remove this schema from the PR. Let's focus only on the runtime configuration schema.

@@ -0,0 +1,129 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

For now, let's remove this schema from the PR. Let's focus only on the runtime configuration schema.

@@ -0,0 +1,313 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

For now, let's remove this schema from the PR. Let's focus only on the runtime configuration schema.

"type": "object",
"additionalProperties": false,
"properties": {
"caliper": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Every root/middle/leaf property must have a user-friendly description, summarizing the property(group) as precisely as possible

eravatee and others added 18 commits October 1, 2022 14:36
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
* Migrate Fabric integration tests to test network

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Address review comments

* Get rid of `rm -fr /tmp/hfc-*`
* Merge redundant phases
* Upgrade SUT version in docker tests
* Remove unnecessary connection profiles
* Made `discover = true`

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Address review comments

* Add connection profiles wherever needed

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
- remove fabric-protos as not needed due to removal of fabric operations
code
- update to latest 2.2 sdk which addresses discovery bug under load

Signed-off-by: D <d_kelsey@uk.ibm.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
* Migrate to npm workspaces

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Address review comments

* Fix npm version in GitHub Actions

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Address review comments

* Define NPM version once in workflow
* Add script for checking prerequisites

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
* Cache node modules across CI workflows

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Address review comments

* Change name of step in integration test workflow
* Remove unnecessary directory creation

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Address review comments

* Add comments
* Remove Fabric 2.4 from cache

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Address review comments

* Fix documentation

Co-authored-by: Dave Kelsey <d_kelsey@uk.ibm.com>
Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

Co-authored-by: Dave Kelsey <d_kelsey@uk.ibm.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
* Execute integration tests based on changes

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Trigger fabric tests

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Trigger ethereum tests

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Trigger fisco-bcos tests

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Trigger through core changes

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Make change that shouldn't trigger any integration tests

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Address review comments

* Fix edge case with empty matrix

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
…dger#1425)

This reverts commit 954eba5.

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
…1427)

* Distinguish different workers in Prometheus PushGateway

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>

* Address review comments

* Remove redundant labels
* Add roundIndex to groupings
* Update unit tests utilizing workerIndex and roundIndex
* Write new unit test for _sendUpdate

Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
Signed-off-by: eravatee <eraraje1997@gmail.com>
@aklenik aklenik merged commit 69000d4 into hyperledger:main Oct 3, 2022
eravatee added a commit to eravatee/caliper that referenced this pull request Oct 4, 2022
…1404)

* JSON schema file for runtime configuration validations
* Basic skeleton for vscode extension to implement further JSON schemas and UI elements

Signed-off-by: eravatee <eraraje1997@gmail.com>
eravatee added a commit to eravatee/caliper that referenced this pull request Oct 4, 2022
…1404)

* JSON schema file for runtime configuration validations
* Basic skeleton for vscode extension to implement further JSON schemas and UI elements

Signed-off-by: eravatee <eraraje1997@gmail.com>
eravatee added a commit to eravatee/caliper that referenced this pull request Oct 4, 2022
…1404)

* JSON schema file for runtime configuration validations
* Basic skeleton for vscode extension to implement further JSON schemas and UI elements

Signed-off-by: eravatee <eraraje1997@gmail.com>
eravatee added a commit to eravatee/caliper that referenced this pull request Oct 4, 2022
…1404)

* JSON schema file for runtime configuration validations
* Basic skeleton for vscode extension to implement further JSON schemas and UI elements

Signed-off-by: eravatee <eraraje1997@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants