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

feat(cactus-plugin-ledger-connector-cdl): add new connector plugin #2962

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

outSH
Copy link
Contributor

@outSH outSH commented Dec 22, 2023

  • Add new plugin for connecting with CDL service.
  • It supports as cactus-plugin-ledger-connector-cdl-socketio which is now depracated and will be removed shortly.
  • There are still no automatic tests for this package, only manual script that user can run against CDL instance to see if everything is working.
  • Fix minor formatting issue in cbdc example (issue from lint)

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

outSH added a commit to outSH/cactus that referenced this pull request Dec 27, 2023
- Remove old connector cactus-plugin-ledger-connector-cdl-socketio. It's been
    replaced with socketio-based cactus-plugin-ledger-connector-cdl.

Depends on hyperledger-cacti#2962

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
sandeepnRES pushed a commit to outSH/cactus that referenced this pull request Jan 4, 2024
- Remove old connector cactus-plugin-ledger-connector-cdl-socketio. It's been
    replaced with socketio-based cactus-plugin-ledger-connector-cdl.

Depends on hyperledger-cacti#2962

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@outSH The bundle validation CI job is failing with this:

/home/runner/work/cacti/cacti/packages/cactus-plugin-ledger-connector-cdl/package.json: 
	dist/cactus-plugin-ledger-connector-cdl.node.umd.min.js
	undefined

/home/runner/work/cacti/cacti/packages/cactus-plugin-ledger-connector-cdl/package.json: 
	dist/cactus-plugin-ledger-connector-cdl.web.umd.min.js
	undefined

@outSH outSH force-pushed the cdl_openapi_connector_pr branch from 9d51b54 to af2ab00 Compare January 18, 2024 10:51
@outSH
Copy link
Contributor Author

outSH commented Jan 18, 2024

@outSH The bundle validation CI job is failing with this:

/home/runner/work/cacti/cacti/packages/cactus-plugin-ledger-connector-cdl/package.json: 
	dist/cactus-plugin-ledger-connector-cdl.node.umd.min.js
	undefined

/home/runner/work/cacti/cacti/packages/cactus-plugin-ledger-connector-cdl/package.json: 
	dist/cactus-plugin-ledger-connector-cdl.web.umd.min.js
	undefined

@petermetz I didn't include these package names because they were not created in the build process. Same seem to apply to all other connector plugins, min bundle creation fails with bunch of errors :/ For now I've added missing package names to please the CI. Please check if bundling is failing on your machine as well or not, and if so, we can create an issue to fix that (and maybe add a script to check if packages were in fact created).

@outSH outSH requested a review from petermetz January 18, 2024 15:57
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@outSH The bundle validation CI job is failing with this:

/home/runner/work/cacti/cacti/packages/cactus-plugin-ledger-connector-cdl/package.json: 
	dist/cactus-plugin-ledger-connector-cdl.node.umd.min.js
	undefined

/home/runner/work/cacti/cacti/packages/cactus-plugin-ledger-connector-cdl/package.json: 
	dist/cactus-plugin-ledger-connector-cdl.web.umd.min.js
	undefined

@petermetz I didn't include these package names because they were not created in the build process. Same seem to apply to all other connector plugins, min bundle creation fails with bunch of errors :/ For now I've added missing package names to please the CI. Please check if bundling is failing on your machine as well or not, and if so, we can create an issue to fix that (and maybe add a script to check if packages were in fact created).

I rebased onto upstream/main, then yarn install, yarn configure, finally I modified the scripts in the package.json to use prod as the webpack env parameter (see screenshot) and then ran yarn build:dev and it worked.
So based on all this I think it is working on my machine. It could be because of some change in the meantime on the main branch.
With all that said, the way you fixed the custom checks is also 100% OK with me, e.g. you declare the min file names but you don't have the scripts defined for the build so the minified webpack build will be skipped anyway. Other packages use the same workaround until we hash out the problems with the minification (which could take a long time so I didn't want us to be slowed down by this kind of issue)

TLDR: The way it is now looks good to me, thank you for the updates!

image

Copy link
Contributor

@izuru0 izuru0 left a comment

Choose a reason for hiding this comment

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

LGTM

- Add new plugin for connecting with CDL service.
- It supports as `cactus-plugin-ledger-connector-cdl-socketio` which is
    now depracated and will be removed shortly.
- There are still no automatic tests for this package, only manual script
    that user can run against CDL instance to see if everything is working.
- Fix minor formatting issue in cbdc example (issue from lint)

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
@petermetz petermetz force-pushed the cdl_openapi_connector_pr branch from af2ab00 to bf45630 Compare January 25, 2024 07:07
@petermetz petermetz enabled auto-merge (rebase) January 25, 2024 07:07
@petermetz petermetz merged commit 6efd8de into hyperledger-cacti:main Jan 25, 2024
127 of 146 checks passed
petermetz pushed a commit to outSH/cactus that referenced this pull request Jan 26, 2024
- Remove old connector cactus-plugin-ledger-connector-cdl-socketio. It's been
    replaced with socketio-based cactus-plugin-ledger-connector-cdl.

Depends on hyperledger-cacti#2962

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
petermetz pushed a commit that referenced this pull request Jan 26, 2024
- Remove old connector cactus-plugin-ledger-connector-cdl-socketio. It's been
    replaced with socketio-based cactus-plugin-ledger-connector-cdl.

Depends on #2962

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.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.

3 participants