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

Expand supported RayError exceptions #221

Merged
merged 32 commits into from
Nov 9, 2023
Merged

Expand supported RayError exceptions #221

merged 32 commits into from
Nov 9, 2023

Conversation

omus
Copy link
Member

@omus omus commented Oct 25, 2023

Creates Julia exceptions to mirror the Ray exceptions supported by Ray for Python.

One challenge with porting thing is that some exceptions are MessagePack serialized containing pickle5 data. As we found out in #191 we can somewhat read the content without having to unpack it. I've opted to show users the repr'd formatted raw data for any exception we haven't seen before. This way users can complain about the bad errors and provide us with examples we can add to our test suites.

There is also some future work to be done around adding in call site information from Julia so we can deserialize that information on the driver and report it to the user. I've opted to leave that for now.

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #221 (6edc777) into main (0b75c77) will increase coverage by 0.22%.
Report is 2 commits behind head on main.
The diff coverage is 98.70%.

@@            Coverage Diff             @@
##             main     #221      +/-   ##
==========================================
+ Coverage   97.42%   97.64%   +0.22%     
==========================================
  Files          12       12              
  Lines         660      808     +148     
==========================================
+ Hits          643      789     +146     
- Misses         17       19       +2     
Flag Coverage Δ
Ray.jl 97.64% <98.70%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/Ray.jl 100.00% <ø> (ø)
src/ray_serializer.jl 100.00% <100.00%> (ø)
src/runtime.jl 94.37% <ø> (ø)
src/exceptions.jl 98.87% <98.70%> (-1.13%) ⬇️

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@omus
Copy link
Member Author

omus commented Nov 3, 2023

Ugh, my merge wasn't so clean as it appeared

@omus omus closed this Nov 3, 2023
@omus omus reopened this Nov 3, 2023
@omus omus marked this pull request as ready for review November 3, 2023 20:47
@omus
Copy link
Member Author

omus commented Nov 3, 2023

Weird formatter failure:

Formatting /home/runner/work/Ray.jl/Ray.jl/build/bind_artifacts.jl
Invalid instruction at 0x7f4e3c60f297: 0x62, 0xf2, 0x7d, 0x48, 0x7c, 0xc0, 0x62, 0xf1, 0x7d, 0x48, 0xfe, 0x0d, 0x59, 0x1e, 0xff

[1909] signal (4.2): Illegal instruction
in expression starting at /home/runner/work/_temp/1d519080-2ee3-48ad-9e23-52a55bb422f8:5
dotop1 at /home/runner/.julia/packages/Tokenize/d4Pxs/src/utilities.jl:45 [inlined]
lex_dot at /home/runner/.julia/packages/Tokenize/d4Pxs/src/lexer.jl:904
next_token at /home/runner/.julia/packages/Tokenize/d4Pxs/src/lexer.jl:380
next_token at /home/runner/.julia/packages/Tokenize/d4Pxs/src/lexer.jl:315 [inlined]
next at /home/runner/.julia/packages/CSTParser/VcYj6/src/lexer.jl:97
#parse_expression#19 at /home/runner/.julia/packages/CSTParser/VcYj6/src/CSTParser.jl:49
parse_expression at /home/runner/.julia/packages/CSTParser/VcYj6/src/CSTParser.jl:41
parse_expression at /home/runner/.julia/packages/CSTParser/VcYj6/src/CSTParser.jl:41
parse_doc at /home/runner/.julia/packages/CSTParser/VcYj6/src/CSTParser.jl:220
parse at /home/runner/.julia/packages/CSTParser/VcYj6/src/CSTParser.jl:248
format_text at /home/runner/.julia/packages/JuliaFormatter/ur5KV/src/JuliaFormatter.jl:239
#format_text#251 at /home/runner/.julia/packages/JuliaFormatter/ur5KV/src/JuliaFormatter.jl:215
format_text at /home/runner/.julia/
...

@omus
Copy link
Member Author

omus commented Nov 3, 2023

3rd run of the formatting workflow worked

Copy link
Member

@kleinschmidt kleinschmidt left a comment

Choose a reason for hiding this comment

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

a few comments. I didn't review this very carefully but let's just merge and wrap this bit up.

src/exceptions.jl Show resolved Hide resolved
src/exceptions.jl Outdated Show resolved Hide resolved
src/exceptions.jl Outdated Show resolved Hide resolved
@omus
Copy link
Member Author

omus commented Nov 9, 2023

Alright, finally going to merge this PR. I need to bump the version yet but I'll let CI finish first.

@omus omus merged commit c89cc85 into main Nov 9, 2023
7 checks passed
@omus omus deleted the cv/more-error-codes branch November 9, 2023 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants