You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/11/26 07:43:28 UTC
[GitHub] [skywalking-nodejs] alanlvle opened a new pull request #65: update grpc
alanlvle opened a new pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65
update:
1.grpc update to @grpc/grpc-js
2.update Authentication Method to realize
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng merged pull request #65: Bump up gRPC version, and use its new release repository
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757291430
##########
File path: scripts/protoc.sh
##########
@@ -24,17 +24,16 @@ mkdir -p $OUT_DIR || true
cd "${ROOT_DIR}"/protocol || exit
PROTOC_GEN_TS_PATH="${ROOT_DIR}/node_modules/.bin/protoc-gen-ts"
-PROTOC_PLUGIN="${ROOT_DIR}/node_modules/.bin/grpc_tools_node_protoc_plugin"
+#PROTOC_PLUGIN="${ROOT_DIR}/node_modules/.bin/grpc_tools_node_protoc_plugin"
Review comment:
This is not the point. The key is removing useless codes, not comment them out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] alanlvle commented on a change in pull request #65: Bump up gRPC version, and use its new release repository
Posted by GitBox <gi...@apache.org>.
alanlvle commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757329393
##########
File path: src/agent/protocol/grpc/AuthInterceptor.ts
##########
@@ -17,23 +17,14 @@
*
*/
-import * as grpc from 'grpc';
-import { InterceptingCall, Listener, Metadata, Requester } from 'grpc';
+import * as grpc from '@grpc/grpc-js';
import config from '../../../config/AgentConfig';
-type Options = { [key: string]: string | number };
-export default function AuthInterceptor(options: Options, nextCall: (options: Options) => InterceptingCall) {
- return new grpc.InterceptingCall(
- nextCall(options),
- new (class implements Requester {
- // tslint:disable-next-line:ban-types
- start(metadata: Metadata, listener: Listener, next: Function) {
- if (config.authorization) {
- metadata.add('Authentication', config.authorization);
- }
- next(metadata, listener);
- }
- })(),
- );
+export default function AuthInterceptor() {
+ const mata = new grpc.Metadata()
+ if(config.authorization){
Review comment:
update 3da6ed9155da8a0aebb36569956adae4bf382a02
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757286451
##########
File path: scripts/protoc.sh
##########
@@ -24,17 +24,16 @@ mkdir -p $OUT_DIR || true
cd "${ROOT_DIR}"/protocol || exit
PROTOC_GEN_TS_PATH="${ROOT_DIR}/node_modules/.bin/protoc-gen-ts"
-PROTOC_PLUGIN="${ROOT_DIR}/node_modules/.bin/grpc_tools_node_protoc_plugin"
+#PROTOC_PLUGIN="${ROOT_DIR}/node_modules/.bin/grpc_tools_node_protoc_plugin"
Review comment:
Why comment out? Rather than deleting?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] alanlvle commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
alanlvle commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757290868
##########
File path: scripts/protoc.sh
##########
@@ -24,17 +24,16 @@ mkdir -p $OUT_DIR || true
cd "${ROOT_DIR}"/protocol || exit
PROTOC_GEN_TS_PATH="${ROOT_DIR}/node_modules/.bin/protoc-gen-ts"
-PROTOC_PLUGIN="${ROOT_DIR}/node_modules/.bin/grpc_tools_node_protoc_plugin"
+#PROTOC_PLUGIN="${ROOT_DIR}/node_modules/.bin/grpc_tools_node_protoc_plugin"
Review comment:
new grpc not use grpc_tools_node_protoc_plugin.
https://www.npmjs.com/package/grpc_tools_node_protoc_ts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757290662
##########
File path: package.json
##########
@@ -69,7 +70,6 @@
},
"dependencies": {
"google-protobuf": "^3.14.0",
- "grpc": "^1.10.1",
Review comment:
It is in `devDependencies` only
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] alanlvle commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
alanlvle commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757289947
##########
File path: package.json
##########
@@ -69,7 +70,6 @@
},
"dependencies": {
"google-protobuf": "^3.14.0",
- "grpc": "^1.10.1",
Review comment:
use "@grpc/grpc-js": "^1.4.4"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#issuecomment-979758866
A lot of format changes, I can't see they are expected, please make sure you submitted the necessary changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] alanlvle commented on pull request #65: Bump up gRPC version, and use its new release repository
Posted by GitBox <gi...@apache.org>.
alanlvle commented on pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#issuecomment-979846001
@kezhenxu94 Yes, what are the changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] alanlvle commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
alanlvle commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757292902
##########
File path: package.json
##########
@@ -69,7 +70,6 @@
},
"dependencies": {
"google-protobuf": "^3.14.0",
- "grpc": "^1.10.1",
Review comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757286627
##########
File path: src/agent/protocol/grpc/AuthInterceptor.ts
##########
@@ -17,23 +17,32 @@
*
*/
-import * as grpc from 'grpc';
-import { InterceptingCall, Listener, Metadata, Requester } from 'grpc';
+import * as grpc from '@grpc/grpc-js';
+// import { InterceptingCall, Listener, Metadata, Requester } from '@grpc/grpc-js';
+// import { type } from 'os';
import config from '../../../config/AgentConfig';
-type Options = { [key: string]: string | number };
+// type Options = { [key: string]: string | number };
-export default function AuthInterceptor(options: Options, nextCall: (options: Options) => InterceptingCall) {
- return new grpc.InterceptingCall(
- nextCall(options),
- new (class implements Requester {
- // tslint:disable-next-line:ban-types
- start(metadata: Metadata, listener: Listener, next: Function) {
- if (config.authorization) {
- metadata.add('Authentication', config.authorization);
- }
- next(metadata, listener);
- }
- })(),
- );
+// function AuthInterceptor(options: Options, nextCall: (options: Options) => InterceptingCall) {
+// return new grpc.InterceptingCall(
+// nextCall(options),
+// new (class implements Requester {
+// // tslint:disable-next-line:ban-types
+// start(metadata: Metadata, listener: Listener, next: Function) {
+// if (config.authorization) {
+// metadata.add('Authentication', config.authorization);
+// }
+// next(metadata, listener);
+// }
+// })(),
+// );
+// }
Review comment:
Same question.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757286310
##########
File path: package.json
##########
@@ -69,7 +70,6 @@
},
"dependencies": {
"google-protobuf": "^3.14.0",
- "grpc": "^1.10.1",
Review comment:
No gRPC dependency?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] kezhenxu94 commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757300150
##########
File path: src/agent/protocol/grpc/clients/TraceReportClient.ts
##########
@@ -53,14 +52,14 @@ export default class TraceReportClient implements Client {
start() {
const reportFunction = () => {
- emitter.emit('segments-sent'); // reset limiter in SpanContext
+ emitter.emit('segments-sent'); // reset limiter in SpanContext
try {
if (this.buffer.length === 0) {
return;
}
- const stream = this.reporterClient.collect((error, _) => {
+ const stream = this.reporterClient.collect(AuthInterceptor(),(error, _) => {
Review comment:
Please add a space after `,`
##########
File path: src/agent/protocol/grpc/AuthInterceptor.ts
##########
@@ -17,23 +17,14 @@
*
*/
-import * as grpc from 'grpc';
-import { InterceptingCall, Listener, Metadata, Requester } from 'grpc';
+import * as grpc from '@grpc/grpc-js';
import config from '../../../config/AgentConfig';
-type Options = { [key: string]: string | number };
-export default function AuthInterceptor(options: Options, nextCall: (options: Options) => InterceptingCall) {
- return new grpc.InterceptingCall(
- nextCall(options),
- new (class implements Requester {
- // tslint:disable-next-line:ban-types
- start(metadata: Metadata, listener: Listener, next: Function) {
- if (config.authorization) {
- metadata.add('Authentication', config.authorization);
- }
- next(metadata, listener);
- }
- })(),
- );
+export default function AuthInterceptor() {
+ const mata = new grpc.Metadata()
+ if(config.authorization){
Review comment:
Please add a space before `(`, after `)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] alanlvle commented on a change in pull request #65: update grpc
Posted by GitBox <gi...@apache.org>.
alanlvle commented on a change in pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#discussion_r757292752
##########
File path: scripts/protoc.sh
##########
@@ -24,17 +24,16 @@ mkdir -p $OUT_DIR || true
cd "${ROOT_DIR}"/protocol || exit
PROTOC_GEN_TS_PATH="${ROOT_DIR}/node_modules/.bin/protoc-gen-ts"
-PROTOC_PLUGIN="${ROOT_DIR}/node_modules/.bin/grpc_tools_node_protoc_plugin"
+#PROTOC_PLUGIN="${ROOT_DIR}/node_modules/.bin/grpc_tools_node_protoc_plugin"
Review comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] kezhenxu94 commented on pull request #65: Bump up gRPC version, and use its new release repository
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#issuecomment-979833751
@alanlvle can you please also update the license information in this directory https://github.com/apache/skywalking-nodejs/tree/master/dist ? Others look good to me, thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on pull request #65: Bump up gRPC version, and use its new release repository
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #65:
URL: https://github.com/apache/skywalking-nodejs/pull/65#issuecomment-979847394
Generally this part, https://github.com/apache/skywalking-nodejs/blob/master/dist/LICENSE#L211. You need to update the LICENSE file according to the latest dependencies, even dependencies of dependencies, which is required by the ASF and Apache 2.0 License.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org