JuliaParallel / ClusterManagers.jl

Other
242 stars 74 forks source link

Elastic auto IP address function #189

Open mikeingold opened 1 year ago

mikeingold commented 1 year ago

I'm interested in adding support for Windows to ElasticManagers addr=:auto feature since it currently only supports Mac and Linux. I checked the source and found that the get_private_ip() function is just running shell commands for these specific platforms and throws an error otherwise. It looks like this functionality was added into Sockets as Sockets.getipaddr() as of Julia v1.2, which probably would've been a compatibility issue back when the get_private_ip() code was authored in 2020, but seems fairly reasonable now that even v1.6 is considered the LTS branch.

I don't mind making some changes and submitting a PR, but this would probably require bumping the Julia compat entry to 1.2, so I figured it would be worth asking the question first: how does the community feel about this? Is this a breaking change for anyone?

kescobo commented 1 year ago

It's really hard to know what's breaking in a package like this one that doesn't really have any tests. I'm personally in favor of adding features and bumping compat to julia 1.6, but am interested in what other people think

mikeingold commented 1 year ago

I went ahead and opened a PR for this update. It's a relatively minor change but does bump compat for Julia to 1.2.