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

Add a -debug flag to enable debug endpoints #308

Closed
wants to merge 1 commit into from

Conversation

quatre
Copy link
Contributor

@quatre quatre commented Apr 3, 2020

This would implement #305 but we can surely improve it as I (lazily, I confess) put many different endpoints under the same flag.

@@ -47,6 +47,7 @@ var (
dumpAst = flag.Bool("dump_ast", false, "Dump AST of programs after parse (to INFO log).")
dumpAstTypes = flag.Bool("dump_ast_types", false, "Dump AST of programs with type annotation after typecheck (to INFO log).")
dumpBytecode = flag.Bool("dump_bytecode", false, "Dump bytecode of programs (to INFO log).")
debug = flag.Bool("debug", false, "Enable debugging endpoints.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Default to true to retain existing behaviour please.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also please make the flag be called "http_debugging_endpoint".

@@ -47,6 +47,7 @@ var (
dumpAst = flag.Bool("dump_ast", false, "Dump AST of programs after parse (to INFO log).")
dumpAstTypes = flag.Bool("dump_ast_types", false, "Dump AST of programs with type annotation after typecheck (to INFO log).")
dumpBytecode = flag.Bool("dump_bytecode", false, "Dump bytecode of programs (to INFO log).")
debug = flag.Bool("debug", false, "Enable debugging endpoints.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Also please make the flag be called "http_debugging_endpoint".

@@ -79,6 +79,7 @@ type Server struct {
dumpAst bool // if set, mtail prints the program syntax tree after parse
dumpAstTypes bool // if set, mtail prints the program syntax tree after type checking
dumpBytecode bool // if set, mtail prints the program bytecode after code generation
debug bool // if set, enable debug endpoints
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable and it's friends should be called httpDebugEndpoints

@@ -299,19 +300,23 @@ func (m *Server) Serve() error {
return errors.Errorf("No bind address provided.")
}
mux := http.NewServeMux()
if m.debug {
mux.Handle("/", m)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the root handler should remain but we can strip the sensitive information in the template.

@@ -107,6 +107,12 @@ func DumpBytecode(m *Server) error {
return nil
}

// Debug enables debug http endpoints
func Debug(m *Server) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

EnableHTTPDebugEndpoints

@jaqx0r jaqx0r added the more-info-needed Waiting for an answer from the issue reporter label Dec 4, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2021

This PR has been waiting for an update for more than 60 days and wlil be closed in 7 if no update is provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-info-needed Waiting for an answer from the issue reporter Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants