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

State of release #216

Closed
ajpauwels opened this issue Apr 12, 2020 · 29 comments
Closed

State of release #216

ajpauwels opened this issue Apr 12, 2020 · 29 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@ajpauwels
Copy link

Hey all, would it be possible to cut a release at the current master commit level or similar? As long as it's post #111 that should work.

Coming in as a new user I was confused between the differing formats in the examples source and in the docs. It didn't help that the examples source (which is the correct way to do things) doesn't compile with the latest version of the crate. Once the dependency is renamed to lambda = { git = "https://github.com/awslabs/aws-lambda-rust-runtime/", branch = "master" } everything works for the new syntax.

I can try and provide documentation and examples improvements PRs but if the maintainers could cut a release I think that'd go a long way to usability.

@outkine
Copy link

outkine commented Apr 23, 2020

Can confirm that using the lambda_runtime dependency (as directed by the README) does not compile because of an issue with an old version of the time library:

error: expected an item keyword
   --> .../time-0.2.8/src/utc_offset.rs:366:13
    |
366 |             let tm = timestamp_to_tm(datetime.timestamp())?;
    |             ^^^

@softprops
Copy link
Contributor

unfortunately the readme has gone stale. I can help with this.

You should only need the lambda crate for basic use. This crate simplified the previous versions and brought support for async/await into the fold. There is some inflight work to bring the lambda-http crate up to speed with the new lambda crate interfaces.

At this point you might be best served by using the examples here adapting the packaging and deployment notes from the readme.

Let us know if you get stuck.

@Luminoth
Copy link

Are there plans to cut a new release tho? Or should we plan to keep using the github dependency for a while still?

I'm also curious when the hello-with-ctx example is going to be expanded on. It isn't super clear to me how that's supposed to work with the macro-based implementation and the example is just copy / paste the hello example right now. Everything else seems more or less clear from the examples and a little bit of trial and error (for instance the examples deal with serde Values for input / output but really they just need to derive the serde traits).

I guess one other question I personally have is - can this be used with proxy API gateways?

@mickdekkers
Copy link

@Luminoth I'm not sure how the context is meant to be accessed, but this seems to work:

use lambda::{lambda, INVOCATION_CTX};
use serde_json::Value;

type Error = Box<dyn std::error::Error + Send + Sync + 'static>;

#[lambda]
#[tokio::main]
async fn main(event: Value) -> Result<(), Error> {
    INVOCATION_CTX.with(|ctx| {
        println!("Context is {:#?}", ctx);
    });
    println!("Event is {:#?}", event);
    Ok(())
}

Here INVOCATION_CTX is a tokio::task::LocalKey<lambda::types::LambdaCtx>.

The downside of this approach is that the context is only available in the closure you pass to with. LocalKey also has a get method but this can't be used at the moment since LambdaCtx doesn't impl Copy (and would also be a bit wasteful, since it would copy LambdaCtx).

Note: I tested this with the latest revision, ebc5474 as of this writing.

@drusellers
Copy link

Coming in as a new user I was confused between the differing formats in the examples source and in the docs. It didn't help that the examples source (which is the correct way to do things) doesn't compile with the latest version of the crate. Once the dependency is renamed to lambda = { git = "https://github.com/awslabs/aws-lambda-rust-runtime/", branch = "master" } everything works for the new syntax.

Yup. Definitely lost a day with this. Looking forward to a new release. Also, happy to help with docs, if for no other reason than to help future me. :)

@mlafeldt
Copy link

I also noticed that there's no git tag/release for the latest crate version (v0.2.1).

@algermissen
Copy link

Hi, I am also struggling with this - any news? Has this maybe been abandoned? Or moved to a different repo?

@softprops
Copy link
Contributor

I feel the struggle. What are the current blockers to publishing a new release now that the http crate is in line with the new "core" crate?

@davidbarsky
Copy link
Contributor

Not abandoned; I'm just short on time. I'm in the process of getting the Lambda runtime team to assign headcount/staff to this runtime—not sure when that will happen, but that would give this library some better stewardship and maintainership than I can currently afford to.

I feel the struggle. What are the current blockers to publishing a new release now that the http crate is in line with the new "core" crate?

Docs, I think? I'd like to give them a once-over before we cut a beta.

@softprops
Copy link
Contributor

@davidbarsky few other notes.

I'm prepping for some new releases of packaged workflows for serverless rust plugin and am trying to get official support for aws sam through the pipeline. I've been updating a bunch of getting templates to reference master in prep for a forth coming release. This made me think about a few things we might want before cutting a release

  1. An updated readme! It's been noted a few times that there's some confusion around the readme examples and what everyone's been talking about with async support. It might be helpful to do a readme refresher.

  2. possibly add a migration doc. The transition as been pretty straight forward for me so far. In the most basic case, the transition looks something like this.

[dependencies]
- lambda_runtime = "0.2"
+ tokio = { version = "0.2", features = ["macros"] }
+ lambda = { git = "https://github.com/awslabs/aws-lambda-rust-runtime/", branch = "master"}
-use lambda_runtime::{error::HandlerError, lambda, Context};
+use lambda::handler_fn;
use serde_json::Value;

-fn main() {
-    lambda!(handler)
+type Error = Box<dyn std::error::Error + Sync + Send + 'static>;

+#[tokio::main]
+async fn main() -> Result<(), Error> {
    lambda::run(handler_fn(handler)).await?;
    Ok(())
}

-fn handler(
-    event: Value,
-    _: Context,
-) -> Result<Value, HandlerError> {
+async fn handler(event: Value) -> Result<Value, Error> {
    Ok(event)
}
  1. possibly provide aggregator crate, like tokio. I think you mentioned this in http crate update but it might be helpful to have one lambda crate with a core (currently what's in lambda) and a http crate. Allowing users to opt into features and http and in the future others with a cargo feature flag vs keeping track of different crates. This doesn't feel like a blocker to me and good be a follow up but I wanted to keep the idea floating in the air.

  2. another bonus idea I'd like to though out is a lambda (md?) book, similar to what you see in -other other ambitious projects. This provides a bit more space for a guided narrative than is afforded in the real estate available in a readme.

@softprops
Copy link
Contributor

Oh also and a finalized lambda context api. I feel like there's some room for improvement there

@Veetaha
Copy link
Contributor

Veetaha commented Jun 26, 2020

@davidbarsky @softprops
Hey guys, this is quite disappointing that there hasn't been a release for a year. If documentation is all that it takes to cut a release, would you review and accept a PR from anyone to write it out (README, examples, api docs, etc.)?

@softprops
Copy link
Contributor

softprops commented Jun 26, 2020

I hear you.

I don't think I have of merge permissions but I'm happy to take a look. api docs and and examples on master should reflect the state of current code. What is ultimately going to be useful is a helpful changelog entry for what changed which is alot. A second in the README for unrelated but upcoming changes would also be good. The good news is this current version has a much smaller surface area to cover.

I'm going to take a stab at the trying to find a slightly nicer context api.

@davidbarsky
Copy link
Contributor

I'm sorry for the frustration. I've been working on a release blog post for the new changes, which will include an updated README.md.

@davidbarsky
Copy link
Contributor

@softprops As for the context API, I think an explicit argument is probably best at this point. The task local approach was too out-there.

@softprops
Copy link
Contributor

softprops commented Jun 26, 2020

I've been working on a release blog post

If it would be help I'd be happy to help

I think an explicit argument is probably best at this point.

I knew you'd come around ;)

I actually just tried to make it work by trying to impl Handler for two flavors of HandlerFn, one for F: Fn(A) and one F: Fn(A, LambdaCtx) but rustc wasn't having that.

I then started down the path of type wrapper types HandlerFn and HandlerFnWithContext to get around the issue above and walked away for a bit because I didn't like that you'd have to call a fn like lambda_fn_with_context() to use it and the fact that there were two. It felt bloaty.

Ultimately I think the explicit context argument is probably best because...

  1. it's a familiar api to those already familiar with other lambda runtimes
  2. it leans a little closer to a plain unit testable fn
  3. fn arguments are easier to discover, there's no question how to reference the lambda context when its in your arg list. the rust analyzer will give you an even more helpful hand
  4. it's easy and common it rust to just ignore parameters in cases where you don't need them foo(a: Event, _: LambdaCtx)

I rework this tonight/tomorrow and push up a pull. my circadian is all kinds of messed up right now :)

@softprops
Copy link
Contributor

@davidbarsky If it helps generate movement, can you nudge the the sam folks if you know anyone on the inside that could help push aws/aws-lambda-builders#174 along.

An ideal but unlikely case time-wise would be this lambda runtime release post could include a nice sam deploy just works™ story. That doesn't have to block this but an end to end rust local to prod story for standard aws lambda tools still needs writ! I'm happy to help where I can.

I feel like it might also be useful to do a beta release before posting to catch anything missed

@cakekindel
Copy link

cakekindel commented Jun 26, 2020

I hear you.

I don't think I have of merge permissions but I'm happy to take a look. api docs and and examples on master should reflect the state of current code. What is ultimately going to be useful is a helpful changelog entry for what changed which is alot. A second in the README for unrelated but upcoming changes would also be good. The good news is this current version has a much smaller surface area to cover.

I'm going to take a stab at the trying to find a slightly nicer context api.

if changelog management is something that got dropped (which is completely understandable) you could consider switching to an automated cicd workflow where version bumps, publish, and changelog are driven by conventional commits with a tool like standard-version - I have a version of this up on a repo of mine that i would be happy to share or talk through

@cakekindel
Copy link

At the risk of overwhelming with offers of help from people unfamiliar with the codebase, I would also love to contribute in any way i can - super psyched about the changes just over the horizon here!

@softprops
Copy link
Contributor

Can we list out what blockers remain and maybe open a issue with a task list of blockers remain?

I'm asking because I've finally had some success at capturing the sam teams attention on the pull adding first class support for the lambda runtime aws/aws-lambda-builders#174

Something I anticipate being problematic is that the integration tests over there depend on this crate by way of git source uri. I'd rather have them point to a release on crates.io to minimize potential breakage in their integration tests when the master branch here changes.

@davidbarsky
Copy link
Contributor

@softprops I believe the README.md is incorrect; that is the only remaining blocker for release.

Oh, as an update, we can't use the crates.io publishing workflow for this repository, unfortunately. GitHub Secrets are disabled by AWS and would require a security assessment on our end, which isn't great.

@cakekindel
Copy link

Would it be OK with AWS' security to use a hosted build service like travis or would the same process apply?

@softprops
Copy link
Contributor

I think it was less about a build service an more about secret manager for crates io token. Automating crate publishing doesn't feel like a blocker for this release. It was just an idea to facilitate more frequent releases

@davidbarsky
Copy link
Contributor

I think it was less about a build service an more about secret manager for crates io token. Automating crate publishing doesn't feel like a blocker for this release. It was just an idea to facilitate more frequent releases

I didn't mean it to come off as that, I'm sorry. This was just a handy mechanism to communicate that blocker :).

Would it be OK with AWS' security to use a hosted build service like travis or would the same process apply?

We'd need to run automated releases through AWS specifically. In other words, the secrets need to be stored on AWS-controlled services and machines. Non-AWS ones need a security review and possibly a pentest.

@softprops
Copy link
Contributor

Might be worth carrying over the conversation about automating releases to new gh issue. That conversation might outlast this one

@Veetaha
Copy link
Contributor

Veetaha commented Sep 6, 2020

@davidbarsky, can we enumerate the blockers (maybe with a checklist), it's sad see the release postponed for indeterminate amount of time...

@calavera
Copy link
Contributor

I know there are a bunch of people waiting for a release, so this might be interesting to you:

#274

@jkshtj jkshtj added enhancement New feature or request question Further information is requested labels Jan 21, 2021
@Tehnix
Copy link

Tehnix commented Mar 10, 2021

Might be worth it to link #294 to give context to why this one was closed ☺️

EDIT: I realise that wording sounded a bit passive aggressive, but was definitely not the intention!

@bahildebrand
Copy link
Contributor

@Tehnix you are completely right. That was an oversight on my part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests