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

docs: update deployment README #251

Merged
merged 4 commits into from
Jul 18, 2024
Merged

docs: update deployment README #251

merged 4 commits into from
Jul 18, 2024

Conversation

mmalenic
Copy link
Member

@mmalenic mmalenic commented Jun 6, 2024

Closes #242

It turns out zig is still required, even though cargo-lambda prompts to install it, because the RustFunction construct doesn't support interactivity. Could potentially automate this by simulating 'enter' presses to install zig, but I don't think it's a good idea without user input.

Changes

  • Swap out rust.aws-cdk-lambda for cargo-lambda-cdk because of slightly better automation when installing rustup targets and building release.
  • Update package.json dependencies, including removing @aws-cdk/aws-apigatewayv2-alpha as it's no longer alpha.

@mmalenic mmalenic self-assigned this Jun 6, 2024
@mmalenic mmalenic requested a review from brainstorm June 6, 2024 05:54
@mmalenic mmalenic added documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file labels Jun 6, 2024
@mmalenic mmalenic enabled auto-merge June 6, 2024 05:54
Copy link
Member

@brainstorm brainstorm left a comment

Choose a reason for hiding this comment

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

Could potentially automate this by simulating 'enter' presses to install zig, but I don't think it's a good idea without user input.

An easier way would be to just pipe to which yes... while I see your point, this only runs on our CI so there's no much harm on piping yes if we have it under control? This change is a revert to previous RustFunction construct if I understand correctly? I recall that the newer one had some significant speed benefits for our CI too?

@mmalenic
Copy link
Member Author

mmalenic commented Jun 6, 2024

This change is a revert to previous RustFunction construct if I understand correctly? I recall that the newer one had some significant speed benefits for our CI too?

This is changing it to the newer one, if you mean the RustFunction we are using in orcabus from this PR?

The other one (i.e, the one being replaced in this in this PR) works too, but I think this one is slightly more configurable from a command exec point of view. They should both be the same speed - there was some other issues in orcabus with multiple compilations of the same package which I don't think are happening here anyway.

@brainstorm
Copy link
Member

This change is a revert to previous RustFunction construct if I understand correctly? I recall that the newer one had some significant speed benefits for our CI too?

This is changing it to the newer one, if you mean the RustFunction we are using in orcabus from this PR?

The old one (i.e, the one being replaced in this in this PR) is okay, but I think the new one behaves a bit nicer from a command exec point of view. They should both be the same speed - there was some other issues in orcabus with multiple compilations of the same package which I don't think are happening here anyway.

Ah, okok, sorry, got them mixed, I thought you were reverting here, all good then, merge!

@mmalenic
Copy link
Member Author

mmalenic commented Jun 6, 2024

Sure, thanks. I'll still try piping yes tomorrow and see what happens.

@mmalenic
Copy link
Member Author

mmalenic commented Jun 6, 2024

I'm having trouble getting yes to work. I've added a note about using cargo lambda build to install zig, which I think is okay for now. I'm not sure if it's the best idea to automatically install zig without user input anyway, especially because different platforms will have different options (e.g. using brew/pip/choco).

@brainstorm
Copy link
Member

Carry on Marko, getting rid of zig cc is a gigantic amount of work: cargo-lambda/cargo-lambda#672

@mmalenic mmalenic added this pull request to the merge queue Jul 18, 2024
Merged via the queue into main with commit 85e4ae3 Jul 18, 2024
5 checks passed
@brainstorm brainstorm deleted the docs/update branch July 18, 2024 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs: update deployment docs
2 participants