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

Change startup commands for bun and deno #6531

Merged
merged 15 commits into from
Jul 24, 2023
Merged

Change startup commands for bun and deno #6531

merged 15 commits into from
Jul 24, 2023

Conversation

jim-king-2000
Copy link
Contributor

Copy link
Collaborator

@waghanza waghanza left a comment

Choose a reason for hiding this comment

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

pm2 is not installed in containers.

You will need to add

{{#deps}}
  RUN apk add {{{.}}}
{{/deps}}

{{#bootstrap}}
  RUN {{{.}}}
{{/bootstrap}}

after WORKDIR in deno.Dockerfile

and bun install prior to CMD in bun.Dockerfile

javascript/config.yaml Outdated Show resolved Hide resolved
@jim-king-2000
Copy link
Contributor Author

pm2 is not installed in containers.

Change it to pm2-runtime.

You will need to add
{{#deps}}
RUN apk add {{{.}}}
{{/deps}}

{{#bootstrap}}
RUN {{{.}}}
{{/bootstrap}}

after WORKDIR in deno.Dockerfile

Done.

and bun install prior to CMD in bun.Dockerfile

Done.

@jim-king-2000
Copy link
Contributor Author

Still some errors. Any solution?

Copy link
Collaborator

@waghanza waghanza left a comment

Choose a reason for hiding this comment

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

Sorry, missing some configuration options

     deps:
        - npm
      bootstrap:
        -  npm --location=global install pm2

for bun and deno

javascript/config.yaml Outdated Show resolved Hide resolved
javascript/config.yaml Show resolved Hide resolved
javascript/config.yaml Show resolved Hide resolved
@waghanza waghanza enabled auto-merge (squash) July 21, 2023 10:02
auto-merge was automatically disabled July 21, 2023 10:03

Head branch was pushed to by a user without write access

@jim-king-2000
Copy link
Contributor Author

jim-king-2000 commented Jul 21, 2023

This error:

curl --retry 5 --retry-delay 5 --retry-max-time 180 --retry-connrefused http://`cat javascript/elysia/ip-bun.txt`:3000 -v
* Closing connection -1

It seems that the ip address is not acquired.

javascript/config.yaml Outdated Show resolved Hide resolved
@jim-king-2000
Copy link
Contributor Author

Still not OK. The same issue.

javascript/config.yaml Outdated Show resolved Hide resolved
@jim-king-2000
Copy link
Contributor Author

Good for bun, but deno still fails.

@jim-king-2000
Copy link
Contributor Author

Accroding to the bun startup command, I modify the deno startup command as the following:
nohup pm2-runtime start "deno run --allow-net --allow-read=. app.ts" -i max
Thank god, it works.

@jim-king-2000
Copy link
Contributor Author

What's wrong withstricjs?

@waghanza
Copy link
Collaborator

I don't know. Any request is failing

Stack is

1 |
2 | const c0=app.static['/']['GET'],c1=app.static['/user']['POST'],f=app.router.find.bind(app.router);return function(r){const s=url.indexOf('/',12) + 1;r.query=r.url.indexOf('?',s);const p=r.query===-1?r.url.substring(s):r.url.substring(s,r.query);switch(p){case'':if(r.method==='GET')return c0(r);break;case'user':if(r.method==='POST')return c1(r);break;}const o=f(r.method,p);if(o===null)return;r.params=o;return o._(r);}
                                                                                                                                                                                                                                                 ^
ReferenceError: Can't find variable: url

cc @aquapi

@jim-king-2000
Copy link
Contributor Author

Shall we fix the failure of stricjs?

@waghanza
Copy link
Collaborator

We can try either in this PR or in another, stricjs I'd already failing on master so both solutions are OK for me.

However, since I cannot publish the results this monday, we can wait a little bit to handle a fix.

@jim-king-2000
Copy link
Contributor Author

OK. I'm going to fix stricjs.

@jim-king-2000
Copy link
Contributor Author

The case of low-http-server failed. I didn't touch it. So I think this PR is ready to be merged.

@waghanza waghanza enabled auto-merge (squash) July 24, 2023 05:51
@waghanza
Copy link
Collaborator

Could you merge master please ? @jim-king-2000

auto-merge was automatically disabled July 24, 2023 06:51

Head branch was pushed to by a user without write access

@jim-king-2000
Copy link
Contributor Author

jim-king-2000 commented Jul 24, 2023

Could you merge master please ? @jim-king-2000

Done.

@waghanza waghanza enabled auto-merge (squash) July 24, 2023 07:27
@waghanza waghanza merged commit 8463ca9 into the-benchmarker:master Jul 24, 2023
39 checks passed
@jim-king-2000
Copy link
Contributor Author

I'm looking forward to the result.

@waghanza
Copy link
Collaborator

Will be done next monday

@jim-king-2000
Copy link
Contributor Author

Will be done next monday

Could be a little bit earlier?

@waghanza
Copy link
Collaborator

I can understand the impatience, but no reasons to rush

I prefer to keep the same publishing day (except that I can not this one)

@jim-king-2000
Copy link
Contributor Author

Sure. Take your time.

By the way, I'm just destroyed by this book.

@jim-king-2000
Copy link
Contributor Author

jim-king-2000 commented Aug 1, 2023

Will be done next monday

No new result? @waghanza

@waghanza
Copy link
Collaborator

waghanza commented Aug 1, 2023

Something went wrong on my end. Will publish results next week (and ping you)

@cyrusmsk
Copy link
Contributor

cyrusmsk commented Aug 1, 2023

I hope that it was not my D PRs) looking forward results next week :) thanks @waghanza

@jim-king-2000
Copy link
Contributor Author

Something went wrong on my end. Will publish results next week (and ping you)

Just a kind reminder, "next week" is coming.

@waghanza
Copy link
Collaborator

waghanza commented Aug 4, 2023

don't worry, it's planned :-)

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