updated to HTTP 1.51; duplicated 0.9.17's RequestHandlerFunction in H…

[mind6]

…ttpHelpers/handlers.jl

HTTP 1 removed function wrapper types like RequestHandlerFunction, the only one used by Dash. It was simple to duplicate 0.9's dispatch functionality and I have put it in Dash.HttpHelpers

[mind6]

CircleCI seems configured to run Julia 1.5, but updating to HTTP 1 needs Julia 1.6 or later.

[etpinard]

Hi @mind6 - thanks very much for this PR! 🎉

I got the tests to pass on locally using Julia 1.8.3 ✔️

Dropping support for Julia 1.5 is probably the way forward. HTTP.jl has done so here JuliaWeb/HTTP.jl#783 about a year ago now.

Strictly speaking though this would imply a breaking change for Dash.jl, Meaning that the next Dash.jl release would be v2.0.0. I'm not sure if Plotly has bigger plans for Dash.jl v2. Perhaps only bumping HTTP.jl compat and dropping Julia 1.5 would be somewhat of an underwhelming v2 release for some.

So, it might end up being easier and faster to:

1. keep the julia requirement at "1.3",
2. set the HTTP compat at HTTP = "0.8.10, 0.9, 1" and
3. have your HttpHelpers.handle function wrap over the behaviour of HTTP of versions <1 and of 1.x

I think something like this

using HTTP
using TOML

const HTTP_VERSION = let
    project = TOML.parsefile(joinpath(pkgdir(HTTP), "Project.toml"))
    VersionNumber(project["version"])
end

@static if HTTP_VERSION < v"1"
    RequestHandlerFunction = HTTP.RequestHandlerFunction
else
    struct RequestHandlerFunction <: HTTP.Handler
        func::Function # func(req)
    end
end

might get us close.

[mind6]

OK. Sounds good.

Should we configure CircleCI to test several Julia versions, such as 1.5, 1.6.7, 1.8.3, nightly build simultaneously? I've seen projects do that using Appveyor.

[etpinard]

Should we configure CircleCI to test several Julia versions, such as 1.5, 1.6.7, 1.8.3, nightly build simultaneously?

That would be nice.

Right now though, the tests are run inside a docker container:

Dash.jl/.circleci/config.yml

Line 8 in f0606e0

- image: plotly/julia:ci

I suspect this makes the dash integration tests (which need a headless browser) much easier to setup.

I'm not sure how tricky it would be to update that docker container such that we can run the Julia ] test inside it. I'm not sure I have push rights to the Plotly dockerhub account.

Perhaps a better way forward would be to setup a Github workflow (using e.g. setup-julia) to run ] test with multiple Julia versions while keeping the docker container just for the integration tests.

[ueliwechsler]

Hi All,
I would love to have this PR accepted in order to be able to use HTTP.jl version > 1 together with Dash.
What is missing in order to merge this PR?

[etpinard]

Hi @ueliwechsler,

The current PR isn't compatible with the lowest Julia version target for Dash.jl which is currently defined as "1.3" (note that Dash.jl is actually only compatible with Julia versions "1.4" and up, see #176 for the details). So, merging this PR as is would be a breaking change and lead to a somewhat underwhelming Dash.jl v2 release.

To workaround that, we would need to write some facade code as suggested in #175 (comment), to support HTTP.jl version "0.9" and those above "1.0".

Moreover, as discovered in #176, the Percy CI tests are currently broken. Figuring out how to get them working again would require some time, unless we can get help from folks at Plotly with Percy experience. I guess we could ignore those failing Percy tests momentarily to merge a HTTP.jl compat PR, but ideally we should fix the Percy tests before doing any other maintenance task in Dash.jl.

Happy new year!

[ueliwechsler]

Hi @etpinard

Thanks a lot for your detailed explanation and the great work with Dash.jl. Makes sense. Is there a way I can help out? Unfortunately, I don't have any experience with Percy CI.

[etpinard]

Is there a way I can help out?

If you could try making the Julia ] test pass under Julia 1.4, that would be awesome 😄

[ueliwechsler]

Is there a way I can help out?

If you could try making the Julia ] test pass under Julia 1.4, that would be awesome 😄

I can try 😅. For the branch corresponding to this PR (mind6:dev or for master?

[etpinard]

For the branch corresponding to this PR

Yes, for the branch corresponding to this PR. The Julia ] test (i.e. not the Percy tests) pass fine on the master and dev branches. It's the the HTTP.jl version bump that's causing issues with Julia 1.4.

[joshday]

@etpinard are you aware that the long term support release of Julia is now 1.6.7? See https://julialang.org/downloads/#long_term_support_release. As someone who maintains a lot of Julia packages and as someone who works with government customers using restrictive computing environments, my estimate of the percentage of Julia users on pre-LTS versions is closer to 0% than 1%. My (unsolicited but well-intended) advice is to drop support for Julia versions older than 1.6.7.

I also don't think bumping the lower bound on Julia constitutes a major version bump according to semver, since there are no changes in the public API.

[etpinard]

Thanks for the input @joshday !

Let me reiterate, I don't personally think it's particularly important to keep compat down to Julia 1.4, but still dropping support for a Julia version is a breaking change. So we would need to bump the Dash.jl version to 2.0.

If someone over at Plotly (cc @alexcjohnson ) is fine with bumping the Dash.jl version to 2.0 (and make a somewhat unspectacular 2.0 release) after merging this PR, then sure 👍

[alexcjohnson]

@etpinard I don't know the Julia ecosystem that well, but to give a little perspective from Dash and Python: We waited for a major version bump to drop Python 2 support because that was a pretty big change, and we haven't dropped support for any minor Python versions since then. However it's very common for Python packages to drop support for older Python versions - minor versions at least - and I think there's a good reason for it: you're not going to break anyone's app with this upgrade, because if you're on a now-unsupported Python version you simply won't get this update. I assume that's the same in Julia, in which case I'd say a minor bump would be fine.

Your call though, if you still feel this requires a major bump I'm not concerned that the release is light.

[mind6]

good point there. If you're on older Julia version you won't see these changes if compat is bumped up.

[etpinard]

Ok great, looks like we're coming to a consensus.

Here's what I think would be the best way forward in preparation for the next Dash.jl release:

• Make a PR that disables the Percy CI tests, to get the CI tests to pass again. Making the Percy tests functional again will be our next priority (cc Fix Percy CI tests #177), but I think Dash.jl users have waited long enough for us not to hold back the release.
• Merge Add Julia test workflow #176, so that we run ] test for newer Julia versions
• Merge this PR here
• Make a release

How does that sound?

[etpinard]

We now (finally) have passing tests on dev 🎉

@mind6 would mind merging the dev branch into your branch?

[etpinard]

Completed in #183 - closing

Thanks again to @mind6 💪 and all the others who made comments +1: