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

Move grpc from devDependencies to dependencies #173

Closed
outSH opened this issue Oct 18, 2023 · 12 comments · Fixed by #182
Closed

Move grpc from devDependencies to dependencies #173

outSH opened this issue Oct 18, 2023 · 12 comments · Fixed by #182
Labels
iroha1

Comments

@outSH
Copy link

outSH commented Oct 18, 2023 •
edited

grpc is a dependency of iroha-helpers (irohaV1) but it's listed under devDependencies instead of dependencies. This causes error when user doesn't have grpc installed. Issue occured at our project (Hyperledger Cacti) when we updated from deprecated grpc to grpc-js.

To reproduce initialize empty TS nodejs project, npm i iroha-helpers (only), compile the following:

import { QueryService_v1Client as QueryService } from "iroha-helpers/lib/proto/endpoint_grpc_pb";

console.log(QueryService.toString());

And run. Error printed:

Error: Cannot find module 'grpc'
Require stack:
- /home/vagrant/tmp/iroha-check/node_modules/iroha-helpers/lib/proto/endpoint_grpc_pb.js
- /home/vagrant/tmp/iroha-check/index.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:995:15)
    at Function.Module._load (node:internal/modules/cjs/loader:841:27)
    at Module.require (node:internal/modules/cjs/loader:1067:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/home/vagrant/tmp/iroha-check/node_modules/iroha-helpers/lib/proto/endpoint_grpc_pb.js:9:12)
    at Module._compile (node:internal/modules/cjs/loader:1165:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1219:10)
    at Module.load (node:internal/modules/cjs/loader:1043:32)
    at Function.Module._load (node:internal/modules/cjs/loader:878:12)
    at Module.require (node:internal/modules/cjs/loader:1067:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/vagrant/tmp/iroha-check/node_modules/iroha-helpers/lib/proto/endpoint_grpc_pb.js',
    '/home/vagrant/tmp/iroha-check/index.js'
  ]
}
This was referenced Oct 18, 2023
Closed
Closed
@petermetz
Copy link
Member

I recommend migrating to the new, purely JS implementation instead because the legacy grpc package has been deprecated for a long time now:

image

@outSH
Copy link
Author

outSH commented Nov 3, 2023

@6r1d @0x009922 Any updates regarding this? It's blocking our CI at the moment. Let us know if this package is still supported and can have grpc fixed / updated in the nearest future. If it's deprecated (or is about to be), then we could remove support for Iroha V1 from cacti.

@0x009922
Copy link
Contributor

Hi @outSH
I cannot help you with the functionality related to Iroha 1, unfortunately.

@6r1d, what about support of Iroha 1 in cacti?

@6r1d
Copy link
Contributor

6r1d commented Nov 13, 2023

@outSH I will update you as soon as I get the decision on that matter.

@outSH
Copy link
Author

outSH commented Nov 23, 2023

@6r1d Thanks for looking into that, any news so far?

@6r1d
Copy link
Contributor

6r1d commented Nov 29, 2023

@outSH, I'm still looking for someone who could help.

@petermetz
Copy link
Member

@6r1d This might help a little bit (but do let me know if it doesn't). The migration from grpc to grpc/grpc-js is meant to be fairly straightforward. What I mean by that is that when I was migrating parts of the Cacti codebase the same way, 99% of the time the only thing I needed to do was to replace the package imports in the code and everything would just work fine out of the box afterwards. There was no need to refactor the code and risk introducing new bugs, etc.

petermetz added a commit to petermetz/iroha-javascript that referenced this issue Dec 2, 2023
@petermetz
build(deps): migrate from grpc to @grpc/grpc-js and make it a prod de…
4e1ca7c 
…pendency

Primary changes:
===============

1. Migrate over from the legacy and unmaintained grpc to @grpc/grpc-js
2. Move from `devDependencies` to `dependencies` so that is a production
dependency of the package.

Secondary changes:
==================

1. Ignored the .yarn/cache directory in git so that there aren't 400
new files in the git index after a `yarn install` execution on the terminal.

Fixes hyperledger#173

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit to petermetz/iroha-javascript that referenced this issue Dec 2, 2023
@petermetz
build(deps): migrate from grpc to @grpc/grpc-js and make it a prod de…
358559c 
…pendency

Primary changes:
===============

1. Migrate over from the legacy and unmaintained grpc to @grpc/grpc-js
2. Move from `devDependencies` to `dependencies` so that is a production
dependency of the package.

Secondary changes:
==================

1. Ignored the .yarn/cache directory in git so that there aren't 400
new files in the git index after a `yarn install` execution on the terminal.

Fixes hyperledger#173

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit to petermetz/iroha-javascript that referenced this issue Dec 2, 2023
@petermetz
build(deps): migrate from grpc to @grpc/grpc-js and make it a prod de…
b55ffa5 
…pendency

Primary changes:
===============

1. Migrate over from the legacy and unmaintained grpc to @grpc/grpc-js
2. Move from `devDependencies` to `dependencies` so that is a production
dependency of the package.

Secondary changes:
==================

1. Ignored the .yarn/cache directory in git so that there aren't 400
new files in the git index after a `yarn install` execution on the terminal.

Fixes hyperledger#173

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit to petermetz/iroha-javascript that referenced this issue Dec 2, 2023
@petermetz
build(deps): migrate from grpc to @grpc/grpc-js and make it a prod de…
bb13547 
…pendency

BREAKING CHANGE: When creating a gRPC Iroha service instance such as
`CommandService` or `QueryService` you have to make sure to create the
credentials object that you will use to construct the service instances
from the new gRPC library. Otherwise your code will throw, claiming that
you have not passed in a valid credentials object (even though you did,
just with the wrong version of the gRPC library)

Primary changes:
===============

1. Migrate over from the legacy and unmaintained grpc to @grpc/grpc-js
2. Move from `devDependencies` to `dependencies` so that is a production
dependency of the package.

Secondary changes:
==================

1. Ignored the .yarn/cache directory in git so that there aren't 400
new files in the git index after a `yarn install` execution on the terminal.

Fixes hyperledger#173

Signed-off-by: Peter Somogyvari <[email protected]>
@petermetz petermetz mentioned this issue Dec 2, 2023
Merged
@petermetz
Copy link
Member

@6r1d I just ended up sending in a quick PR. It's a pretty simple change. I verified that it works with our Cacti Iroha connector tests.
See my PR: #182

@baziorek baziorek added the iroha1 label Dec 2, 2023
@6r1d 6r1d closed this as completed in #182 Dec 2, 2023
@0x009922
Copy link
Contributor

0x009922 commented Dec 4, 2023 •
edited

TBH, I am not sure why this error occurs in runtime, because grpc library is not imported from what I can observe in source code.

However, grpc is imported in a .d.ts file, which means it must be a production dependency, not a dev one.

With this in mind, I've approved #174.

upd: I am blind, just saw #182, nevermind.

@outSH
Copy link
Author

outSH commented Dec 12, 2023

@6r1d @0x009922 Hi, do you plan to release a version containing this fix?

@outSH
Copy link
Author

outSH commented Mar 4, 2024

@6r1d @0x009922 any news regarding releasing new version with that fix? :)

@0x009922 0x009922 mentioned this issue Mar 7, 2024
Closed
@0x009922
Copy link
Contributor

0x009922 commented Mar 7, 2024

@outSH, created a task for it: #189

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
iroha1
Projects
None yet
Milestone
No milestone
5 participants
@petermetz @6r1d @outSH @baziorek @0x009922