You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "promer94 (via GitHub)" <gi...@apache.org> on 2023/03/31 03:26:20 UTC

[GitHub] [incubator-opendal] promer94 opened a new pull request, #1814: chore(bindings/nodejs): add deno benchmark

promer94 opened a new pull request, #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814

   Add benchmark using deno.
   
   ### Notice
   Currently the benchmark cannot run since `@opendal/lib-darwin-arm64` is missing from the npm.


-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] suyanhanx commented on a diff in pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on code in PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814#discussion_r1153989507


##########
bindings/nodejs/benchmark/deno.ts:
##########


Review Comment:
   It's also good as part of the comparison with similar aws-s3 projects.



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] promer94 commented on a diff in pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "promer94 (via GitHub)" <gi...@apache.org>.
promer94 commented on code in PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814#discussion_r1153988454


##########
bindings/nodejs/benchmark/deno.ts:
##########


Review Comment:
   unfortunately `aws-sdk-js-v3` can't be used in deno directly(it also not works in bun). So i have to use another s3 client for deno.  🥲
   
   https://github.com/aws/aws-sdk-js-v3/issues/1289
   https://github.com/oven-sh/bun/issues/2465



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814#discussion_r1153989159


##########
bindings/nodejs/benchmark/deno.ts:
##########


Review Comment:
   > unfortunately `aws-sdk-js-v3` can't be used in deno directly(it also not works in bun).
   
   Aha, good news for opendal! Users who want to visit s3 on deno can use opendal :rocket: 



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] promer94 commented on a diff in pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "promer94 (via GitHub)" <gi...@apache.org>.
promer94 commented on code in PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814#discussion_r1153984818


##########
bindings/nodejs/benchmark/deno.ts:
##########


Review Comment:
   It could run js code. I just prefer TypeScript. 😁
   
   I could convert this file to js if you like



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814#discussion_r1153980614


##########
bindings/nodejs/benchmark/deno.ts:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Operator } from 'npm:opendal'
+import 'https://deno.land/std@0.181.0/dotenv/load.ts'

Review Comment:
   deno is interesting!



##########
bindings/nodejs/benchmark/deno.ts:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Operator } from 'npm:opendal'

Review Comment:
   We must load from `npm`? Maybe it's more useful to test againest local build?



##########
bindings/nodejs/benchmark/deno.ts:
##########


Review Comment:
   I used to think we can run js code on `deno` directly...



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814#discussion_r1153984549


##########
bindings/nodejs/benchmark/deno.ts:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Operator } from 'npm:opendal'

Review Comment:
   Understood. Could you please leave a comment with a link to https://github.com/napi-rs/napi-rs/issues/900 here? This will allow us to migrate once napi is released.



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] suyanhanx commented on a diff in pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on code in PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814#discussion_r1153989507


##########
bindings/nodejs/benchmark/deno.ts:
##########


Review Comment:
   It's also good as part of the comparison with similar aws-s3 client projects.



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo merged pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814


-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] promer94 commented on a diff in pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "promer94 (via GitHub)" <gi...@apache.org>.
promer94 commented on code in PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814#discussion_r1153983309


##########
bindings/nodejs/benchmark/deno.ts:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Operator } from 'npm:opendal'

Review Comment:
   local import is not supported yet. 
   https://github.com/napi-rs/napi-rs/issues/900#issuecomment-1271821940
   
   So i think we could keep it simple for now. Once napi-rs has a batter support for deno ffi, we could opt-in later.



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #1814: chore(bindings/nodejs): add deno benchmark

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #1814:
URL: https://github.com/apache/incubator-opendal/pull/1814#discussion_r1153985591


##########
bindings/nodejs/benchmark/deno.ts:
##########


Review Comment:
   Oh, what I mean is whether we can use the same code for both Node and Deno. I don't have a preference for JS or 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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org