Adding Rust Code with Elixir Bindings #1
Conversation
bradschwartz
force-pushed
the
init-functionality
branch
from
25c88c6 to
b615790
Compare
…s on how to force compilation
| on: | ||
| pull_request: | ||
| branches: | ||
| - master |
There was a problem hiding this comment.
Will this run the release workflow on any pull requests to master? Just making sure that's the desired behavior. Looks like things are only published on a tag, so in other cases is this so one could pull down a built/uploaded artifact and check something manually?
There was a problem hiding this comment.
These will run on PR to master, on master, and on the creation of new tags. Maybe calling it release.yml is a bit of a misnomer since I'm also using it just to check that all the NIFs/rust code compiles. I'm fine with it publishing artifacts on non-tags, but i also don't see a huge gain in doing it. Most of this is copied from https://github.com/philss/rustler_precompiled?tab=readme-ov-file ( or links) .
.github/workflows/release.yml
Outdated
| - master | ||
| push: | ||
| branches: | ||
| - main |
There was a problem hiding this comment.
Will this repo have a "main" branch separate from "master" (which seems to be current default)?
There was a problem hiding this comment.
Nope, just copy-pasta
native/json_schema_nif/src/lib.rs
Outdated
| rustler::atoms! { | ||
| bad_instance, | ||
| bad_schema, | ||
| matches_schema, |
There was a problem hiding this comment.
I don't know if rustler has a good way to do this or if you'd have to wrap it, but in my opinion in feels a little odd to return {:ok, :matches_schema} instead of just :ok
🤷 not a big deal, though
There was a problem hiding this comment.
Locally I see this:
iex(1)> JsonSchemaNif.validate_json(~S/{"age":30}/, ~S/{"type":"string"}/)
{:error, :violates_schema}
iex(3)> JsonSchemaNif.validate_json(~S/{"age":"thirty"}/, ~S/{"type":"object"}/)
{:ok, :matches_schema}So I think we already have the behavior you're asking for?
There was a problem hiding this comment.
II think his point is to return ok only. not a tuple. I could agree with that
There was a problem hiding this comment.
I see, just pushed this change
| let instance_value = match serde_json::from_str::<Value>(&instance) { | ||
| Ok(val) => val, | ||
| Err(err) => { | ||
| error!("Failed to parse JSON: {:?}", err); |
There was a problem hiding this comment.
Are these errors being logged? Are the err values something that would be easy to send to Elixir? I could just see not wanting to log these, so I'm wondering if they could be returned like
{:error, {:bad_schema, "Unexpected char '<'"}} # not sure what err looks likeThere was a problem hiding this comment.
I don't think so, at least I can't seem to make it show up in iex 🤷
.github/workflows/ci.yml
Outdated
| elixir_version: [1.15] | ||
| otp_version: [24, 25, 26] | ||
| exclude: | ||
| - otp_version: 26 |
There was a problem hiding this comment.
can remove all exclusion here.. remove OITP 24
lets just do 1.15, 1.16, and then OTP 25 and 26
| @@ -0,0 +1,5 @@ | |||
| { | |||
There was a problem hiding this comment.
We don't totally need it, but without it vscode fails to recognize that there is also a rust project in this codebase. If you feel strongly, i'm happy to remove
CHANGELOG.md
Outdated
|
|
||
| ## [Unreleased] | ||
|
|
||
| ## [0.1.0] - 2023-11-17 |
renovate.json
Outdated
| @@ -0,0 +1,4 @@ | |||
| { | |||
There was a problem hiding this comment.
are we not using dependabot here? that s the default on github
renovate.json
Outdated
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "$schema": "https://docs.renovatebot.com/renovate-schema.json", | |||
| "extends": ["local>engineering/ops/renovate-config"] | |||
There was a problem hiding this comment.
this is only for gitlab. I suspect
There was a problem hiding this comment.
Yep good call, will scrub from history
Adds all the initial functionality. API looks like:
On PRs, we'll compile the Elixir project with
RUSTLER_PRECOMPILATION_FORCE_BUILD=true. For some weird reason I couldn't get this to compile on Ubuntu 20.04 with Elixir 1.16 and OTP 25 - other OTP versions worked fine. See this Run for example error: https://github.com/podium/json_schema_nif/actions/runs/8913023798. For this reason, just running onubuntu-latestfor the Elixir CI. We also run the NIF compilation on PRs, but only push the artifacts on tag creation.Currently, not set up to run any sort of Rust specific checks like formatting or
clippy.MIT License.