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

[Go] Update expose to allow mounting routes from imported packages #39

Merged
merged 4 commits into from
Jan 30, 2023

Conversation

ewucc
Copy link
Contributor

@ewucc ewucc commented Jan 6, 2023

Adds support for router.Mount() which allows you to import routers from other packages within your application.

r.Get("/", func(w http.ResponseWriter, r *http.Request) {
	w.Write([]byte("Hello from Klotho!"))
})

r.Mount("/extra", routes.ExtraRoutes())

Updated Docs PR: https://github.com/klothoplatform/docs/pull/135
Updated Sample App PR: https://github.com/klothoplatform/GoSampleApps/pull/1/files

Standard checks

  • Unit tests: Any special considerations?
  • Docs: Do we need to update any docs, internal or public?
  • Backwards compatibility: Will this break existing apps? If so, what would be the extra work required to keep them working?

@github-actions
Copy link

github-actions bot commented Jan 6, 2023

Package Line Rate Health
github.com/klothoplatform/klotho/pkg/analytics 2%
github.com/klothoplatform/klotho/pkg/annotation 24%
github.com/klothoplatform/klotho/pkg/core 21%
github.com/klothoplatform/klotho/pkg/env_var 82%
github.com/klothoplatform/klotho/pkg/exec_unit 41%
github.com/klothoplatform/klotho/pkg/infra/kubernetes 58%
github.com/klothoplatform/klotho/pkg/infra/kubernetes/helm 52%
github.com/klothoplatform/klotho/pkg/input 63%
github.com/klothoplatform/klotho/pkg/lang 37%
github.com/klothoplatform/klotho/pkg/lang/dockerfile 0%
github.com/klothoplatform/klotho/pkg/lang/golang 19%
github.com/klothoplatform/klotho/pkg/lang/javascript 46%
github.com/klothoplatform/klotho/pkg/lang/python 60%
github.com/klothoplatform/klotho/pkg/lang/yaml 0%
github.com/klothoplatform/klotho/pkg/logging 7%
github.com/klothoplatform/klotho/pkg/multierr 95%
github.com/klothoplatform/klotho/pkg/provider/aws 60%
github.com/klothoplatform/klotho/pkg/runtime 75%
github.com/klothoplatform/klotho/pkg/static_unit 32%
github.com/klothoplatform/klotho/pkg/validation 73%
Summary 42% (3661 / 8714)

// For external routes, we work back from the mount() call to get the package being called. Then
// we find the function which defines the extra routes within the specified package

routerMounts := h.findChiRouterMounts(f, routerName)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is still limited to the same file as the listen call right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when checking for external calls, we no longer care about the listen. So we're just getting the package name of the mounted package and finding the file where the routes are defined. It's no longer in the main.go file w/ the listen anymore. Not sure if that answered your question

Copy link
Contributor

Choose a reason for hiding this comment

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

this is only run when a file has annotations though and weve found the listener. Im not sure if go allows the routes to be in a separate file, but is that working? Im not sure if your answer is clear to me, im a little confused


}

return errors.Errorf("No import package found with name or alias [%s]", mount.PkgAlias)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to be returning errors from this plugin? Like at this point are we certain they meant to match the go chi syntax for us? If we ever supported another expose library for go would this get confusing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeh i was kind of on the fence. I didn't put as many errors in the first time around since we could technically just silently continue if we don't find what we expect. At this point though, yeh we have determined that there is a mount call to an external package, but we cannot find the package in the imports. Though if this were to happen, the go build for the project just wouldn't work either.

Copy link
Contributor

Choose a reason for hiding this comment

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

alright yeah thats fine then i guess, can always remove it

@ewucc ewucc merged commit 8e0b502 into main Jan 30, 2023
@ewucc ewucc deleted the go-expose-routes branch January 30, 2023 22:02
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.

2 participants