JarroVGIT commented on PR #1852:
URL: 
https://github.com/apache/datafusion-ballista/pull/1852#issuecomment-4664489831

   Alright it compiles again. It was quite interesting to walk through the 
entire codebase and I ran into several places where I had to dig around to find 
out if `HashMap<String, HashMap<String, SomeType>>` contained a job id. The 
following was the most interesting (to me):
   
   - [scheduler - 
state/aqe/planner.rs](https://github.com/apache/datafusion-ballista/blob/fe36e9301e68c22e5b63c01eeb59f4a9698deb67/ballista/scheduler/src/state/aqe/planner.rs#L167-L169):
 This calls a function that takes a `job_id` parameter, but a `job_name` is 
passed.
   - [scheduler - 
scheduler_server/event.rs](https://github.com/apache/datafusion-ballista/blob/0e897e86a877b71adf578a21e1d8536283d551af/ballista/scheduler/src/scheduler_server/event.rs#L153-L155):
 The composed string claims this variant contains a job id, while the call 
sites only populate it with an executer id.
   - Some signatures take values by owned value, others take it by reference. 
In some cases, I'd argue that we could do a bit of a clean up. For example, any 
`Struct::new()` should take parameters that are going to be owned values in the 
struct as owned values, not as references. (See also 
[here](https://rust-lang.github.io/api-guidelines/flexibility.html#caller-decides-where-to-copy-and-place-data-c-caller-control)
 in the Rust API guidelines).
   - Although it was a few hours work (of which I am sure most of it could've 
been automated or LLM-ed), it was relatively easy to just (1) change struct 
fields with find+replace, (2) change signatures with find+replace and (3) fix 
all compiler errors. In the last step you'll eventually run into the places 
where it actually began for me; the `HashMap<String, _>` and such. It was quite 
fun, actually.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to