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

cli/daemon: database improvements #113

Merged
merged 3 commits into from
Dec 6, 2021

Conversation

horiajurcut
Copy link
Contributor

@horiajurcut horiajurcut commented Dec 6, 2021

Fixes #107 #98 #49

@encore-cla
Copy link

encore-cla bot commented Dec 6, 2021

All committers have signed the CLA.

Copy link
Member

@eandre eandre left a comment

Choose a reason for hiding this comment

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

Looks good! One minor nitpick but otherwise ready to ship.

@@ -236,38 +236,54 @@ func (s *Server) Test(req *daemonpb.TestRequest, stream daemonpb.Daemon_TestServ
}
s.cacheAppRoot(man.AppID, req.AppRoot)

setupCtx, setupCancel := context.WithTimeout(context.Background(), 30*time.Second)
var usesSQLDB bool
for _, svc := range parse.Meta.Svcs {
Copy link
Member

Choose a reason for hiding this comment

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

This can be replaced with usesSQLDB := requiresSQLDB(parse.Meta)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missed it! Thanks. It's replaced now

Copy link
Member

@eandre eandre left a comment

Choose a reason for hiding this comment

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

Nice!

@eandre eandre merged commit a7f3185 into encoredev:main Dec 6, 2021
horiajurcut added a commit to eandre/encore that referenced this pull request Dec 14, 2021
horiajurcut added a commit to horiajurcut/encore that referenced this pull request Dec 21, 2021
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.

Quiting db proxy don't close assigned port
2 participants