JarroVGIT opened a new issue, #1876: URL: https://github.com/apache/datafusion-ballista/issues/1876
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.** With the landing of #1852 , the next logical place to create a new type for is `ExecutorId`. This will help making clear when collections are keyed by executor id's or job id's, for example. The approach is similar (or, the same really) as for `JobId`. **Describe the solution you'd like** - Create a new type `ExecutorId(String)` (or `ExecutorId(Uuid)`) - Replace all `executor_id` with this new type. **Describe alternatives you've considered** Executor ID's are UUIDs, and it was suggested to making it `ExecutorId(Uuid)` (see this [comment](https://github.com/apache/datafusion-ballista/pull/1852#issuecomment-4673754250)). There are a few downsides to this that I could think of: - We would have to change a lot of tests. - Keeping `ExecutorId` semantically an opaque identifier (as it is now as well) feels more flexible. - We wouldn't be able to reuse the introduced macro that automatically implements the ergonomic (imo, obviously biased as author) traits such as `From<&str>` and `Borrow<str>`. The latter is important for easy use with `HashMap`s. I could of course make a new macro so this is not a very strong argument. @martin-g : as the original commenter, what do you think? If you think `ExecutorId(Uuid)` makes more sense, then I will do that. **Additional context** This will await #1851 as that work is in all the same places. -- 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]
