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

chore(auraescript): Upgrade Deno #513

Merged
merged 3 commits into from
Jul 8, 2024

Conversation

mccormickt
Copy link
Contributor

Fixes some build errors due to dependency conflicts by upgrading Deno libraries. We also split deno_runtime and deno_core again to allow for macros to work properly (re-exported deno_core could not be found).

Also disables the snake case clippy lint for auraescript modules as it now fails on the as__ prefixed functions generated by our macros.

@mccormickt mccormickt force-pushed the mccormickt/update-deno branch 3 times, most recently from ccc5b33 to d550fb8 Compare June 23, 2024 23:18
@mccormickt mccormickt marked this pull request as draft June 25, 2024 03:59
@mccormickt
Copy link
Contributor Author

Seems like this breaks auraescript, converting to draft to figure it out. The usual issue of the ops not being registered properly.

» ./examples/cells.ts 
Error: TypeError: Deno[Deno.internal].core.ops.as__aurae_config__try_default is not a function
    at Module.createClient (file:///home/jan0ski/aurae-runtime/aurae/auraescript/gen/aurae.ts:15:47)
    at file:///home/jan0ski/aurae-runtime/aurae/examples/cells.ts:4:28

@mccormickt mccormickt force-pushed the mccormickt/update-deno branch from d550fb8 to 254b5cd Compare June 25, 2024 21:31
@mccormickt mccormickt marked this pull request as ready for review June 26, 2024 01:40
@mccormickt
Copy link
Contributor Author

mccormickt commented Jun 26, 2024

Seems to be working well! A few errors that seem to be more like bugs in auraed not cleaning up cgroups/executables or in the scripts themselves rather than in auraescript:

» cargo run -p auraescript ./examples/discovery_discover.ts
{
  "healthy": true,
  "version": "0.0.0"
}
» cargo run -p auraescript ./examples/cells_isolated_processes_and_network.ts 
{
  "cellName": "ae-net-proc-isolated-1",
  "cgroupV2": true
}
{
  "pid": 19
}
{
  "pid": 21
}
{
  "pid": 25
}
Error: Error: status: Internal, message: "cell 'ae-net-proc-isolated-1' could not kill children: No child process (os error 10)", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "date": "Wed, 26 Jun 2024 01:41:53 GMT", "content-length": "0"} }
    at async file:///home/jan0ski/aurae-runtime/aurae/examples/cells_isolated_processes_and_network.ts:84:1


@dmah42
Copy link
Contributor

dmah42 commented Jun 26, 2024

interesting. do we see this before the upgrade?

@mccormickt
Copy link
Contributor Author

interesting. do we see this before the upgrade?

After a restart these "no process found" or "cgroup exists" errors are gone (both on main and this branch). The problem seemed to be stale cgroups on my machine from prior tests not being cleaned up 👍

@dmah42
Copy link
Contributor

dmah42 commented Jul 2, 2024

interesting. do we see this before the upgrade?

After a restart these "no process found" or "cgroup exists" errors are gone (both on main and this branch). The problem seemed to be stale cgroups on my machine from prior tests not being cleaned up 👍

so is this ready to merge?

let path = module_specifier
.to_file_path()
.map_err(|_| anyhow!("Only file: URLs are supported."))?;
.map_err(|_| anyhow!("Only file:// URLs are supported."))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

should we use the requested module type instead of extracting it from the path? or maybe use the requested module path and the path-derived version of None is passed in?

@dmah42 dmah42 merged commit 6de0eab into aurae-runtime:main Jul 8, 2024
17 checks passed
@mccormickt mccormickt deleted the mccormickt/update-deno branch July 9, 2024 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants