Gabor Javorszky

Variable fqfn conflicts with imported package

5

In vkrouter * import: https://github.com/suborbital/atmo/blob/main/atmo/appsource/vkrouter.go#L11 * variable: https://github.com/suborbital/atmo/blob/main/atmo/appsource/vkrouter.go#L73 This should not happen. Gets confusing, and go's shadow rules will make it hard to understand which one is being used.

Variable url conflicts with imported package

1

* import: https://github.com/suborbital/atmo/blob/main/atmo/appsource/httpsource.go#L8 * variable: https://github.com/suborbital/atmo/blob/main/atmo/appsource/httpsource.go#L252 This can lead to heisenbugs due to shadowing.

problems variable name shadows / conflicts with struct name

3

It's in the directive validator: https://github.com/suborbital/atmo/blob/main/directive/directive_validator.go#L16

Atmo doesn't seem to have graceful shutdown implemented in the codebase. The wrapped vk router has stop and stopctx methods on it, but they're not being used anywhere. Because of...

This is on the heels of making atmo multiple app source capable Starting comment: https://github.com/suborbital/atmo/pull/121#discussion_r808574493 Each different application on a different domain should have its own router, because if both...

The interface shouldn't care about authorization headers at all, that is an implementation level detail. Currently there are 3 implementations for the appsource interface: * bundle, doesn't need it *...

https://github.com/stretchr/testify#suite-package provides a really nice framework to building test suites I've ran into the init in tests while trying to figure out why the atmo tests were failing, specifically this...

code: https://github.com/suborbital/atmo/blob/main/atmo/coordinator/coordinator.go#L148 Not sure whether the database connections would need any sort of special treatment, but if they don't, at least a blurb in the code comment would be nice.

resp body needs to be closed at all times

1

In the httpsource code, the response body is only closed when the dest, one of the input parameters, is not nil, but the request that produces the response body gets...

https://github.com/suborbital/atmo/blob/main/atmo/appsource/httpsource.go#L266 Generally all requests should have a timeout set on them. If there isn't one, like in this case, there is no timeout, so a request could be pending indefinitely...