Skip to content

unresolved import lambda_attributes when compiling with default-features = false #259

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

Closed
mikeyhew opened this issue Sep 16, 2020 · 8 comments
Labels
bug Something isn't working

Comments

@mikeyhew
Copy link

mikeyhew commented Sep 16, 2020

If I use this crate without any defualt features enabled, it fails to compile because something from the lambda_attributes crate is imported, even though it isn't a dependency in Cargo.toml. It should be a simple fix - just add a #[cfg(feature = "lambda-attributes")] above the import, and maybe in a few other places.

In order to avoid regressions in the future, we could update the hello-no-macro example to use the lambda crate without any default features, so that it fails to compile if this breaks again (and hopefully causes CI to fail).

To reproduce, use this in your Cargo.toml file:

lambda = { git = "https://github.com/awslabs/aws-lambda-rust-runtime", default-features = false}
@davidbarsky
Copy link
Contributor

Thanks for reporting this, and you're right that this is an issue. More broadly, I'm unhappy with lambda-attributes and I'd like to remove it for two reasons:

  • The compiler errors people get from it are bad.
  • Due to how cold/warm starts work on Lambda, the naive approach of setting up resources (such as loggers) within a #[lambda]-decorated block will cause the logger initializer to panic in a warm start. That's not great.

What do you think?

@softprops
Copy link
Contributor

I feel like it's still useful. It's just a matter of documentation https://awslabs.github.io/aws-lambda-rust-runtime/lambda_http/#hello-world-without-macros

@mikeyhew
Copy link
Author

I wouldn't be opposed to that. There's a lot of work involved in building the actual function for the x86_64-unknown-linux-musl target, setting up the appropriate roles and permissions, and creating the function on AWS; and the ~5 lines of code that the macro saves you isn't really a whole lot in the grand scheme of things.

I actually did try using it at first, but as soon as I saw the error message issue, I got rid of it. The reason I wanted to disable it with default-features = false is because it requires on an old version of proc-macro2 that I didn't want to add to my Cargo.lock.

One suggestion: you could always move it out to a separate crate, and have someone else maintain it, at least for now until the error message issue is resolved.

@rimutaka
Copy link
Contributor

The other negative of having 2 pathways is the initial confusion. It takes some time to understand the difference. Should I use main or macro? What are the pro's and con's? It would be less code to maintain too if we got rid of it.

@softprops
Copy link
Contributor

Should I use main or macro? What are the pro's and con's?

See the docs linked above it specifically calls out when not to use it and the tradeoff of when you are using it

@davidbarsky
Copy link
Contributor

See the docs linked above it specifically calls out when not to use it and the tradeoff of when you are using it

I think that the primary upside of the #[lambda] macro is that "it looks nice in a screenshot". However, the downsides are severe when used in a typical production setting.

The #[lambda] macro introduces a discontinuity for the customer as they learn this crate's API—they must graduate to the desugared lambda::run-based API in order to make their Lambda function(s) operationally ready by introducing logging, metrics, and distributed tracing. Worse, the customer might not discover the downsides of the #[lambda] API until they've deployed their application to production and are seeing warm invocations. I'd rather remove this API than risk impacting people's production stages.

@miere
Copy link

miere commented Sep 22, 2020

That’s absolute spot on, @davidbarsky. Having less lines gives the idea that it is so simple it might cover all the side effects.

@bahildebrand
Copy link
Contributor

#282 removed the lambda attributes crate, so this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants