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.

Sign up for GitHub

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

Jump to bottom

Attempt at bringing gen_resources back to life #208

Closed
wants to merge 18 commits into from
Closed

Attempt at bringing gen_resources back to life #208

wants to merge 18 commits into from

Conversation

etpinard
Copy link
Collaborator

My attempt at updating the 18+ months old https://github.com/plotly/DashCoreResources tarball.

I do not have push rights to https://github.com/plotly/DashCoreResources, so I pushed to my fork https://github.com/etpinard/DashCoreResources, see diff.

I needed to tweak the Sources.toml in a8f15a2 as the dash renderer is no long a separate python package.

This isn't quite sufficient though. Using the new artifacts, I get

image

Any help here would be much appreciated!

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jun 13, 2023
@etpinard etpinard added priority Dash.jl tasks that should be completed prior to non-priority items and removed documentation Improvements or additions to documentation labels Jun 13, 2023
@etpinard
Copy link
Collaborator Author

etpinard commented Jun 13, 2023 •
edited
Loading

In the generated DashCoreResources/resources/dash_renderer.yaml, these lines

image

could be telling us something 🤔

@alexcjohnson
Copy link
Contributor

hmm something to do with the refactoring we did with Python Dash 2.0? I didn't think that moved the renderer but maybe it had some side-effect. Anyway I gave you write access to https://github.com/plotly/DashCoreResources, if that's still useful.

etpinard added 2 commits June 14, 2023 11:44
@etpinard
add newline at EOF
afacbc4
@etpinard
tweak dash_renderer entry in main_registry
56a0ae2 
we need to override the `namespace` value
with `"dash_renderer" to fetch from the
correct URL

we do something similar for the renderer deps
just above.
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jun 14, 2023
@etpinard
Copy link
Collaborator Author

I got the resources to load properly with 56a0ae2, there might a better way to do this.

Briefly, we need to account for the differences between the old dash_renderer_deps file tree and the new one. The generated dash_renderer.yaml diff (see #208 (comment)) get us close, but not quite to the finish line.

This results in two failing integration tests

image

to be continued ....

@etpinard etpinard removed the documentation Improvements or additions to documentation label Jun 20, 2023
@github-actions github-actions bot added documentation Improvements or additions to documentation tests labels Jun 21, 2023
etpinard added 6 commits June 21, 2023 15:39
@etpinard
add newline at EOF
a4b6b41
@etpinard
adapt clientside callback with Promise test
6eb9c06 
clientside callbacks with Promises are now supported
by dash since v2.4.0
@etpinard
adapt devtools test_prop_check suite
95d0561 
`invalid-nested-prop` became `allow-nested-prop`
in dash 2.1.0

moreover, use same css query as in the python
version of the `test_props_check.py` suite
@etpinard
add instructions on how to run the integration tests
50fe3d4 
in the same docker container as on CI
@etpinard
add dash-main to gitignore
6de0bd5 
the `dash-main` folder the repo root
is used during the integration tests
to install the python testing deps
@etpinard
lint and improve integration test README
dc32610
@etpinard etpinard mentioned this pull request Jun 21, 2023
Open
@etpinard
Copy link
Collaborator Author

Update

I got the integration tests to pass again 🎉

  • commit 6eb9c06 adapts the Promise in clientside callback test as now JS Promises do work inside clientside callbacks thanks to dash#2009, released in dash v2.4.0
    • It might be nice to adapt and include the other integration tests added in dash#2009 in Dash.jl while at it
  • commit 95d0561 adapts the devtools prop check test suite (essential what this piece of a very large patch did in python) now that nested props are valid since dash#1763, released in dash v2.1.0

Now, the percy snapshots did generate two diffs:

image

image

which I'm not 100% sure were expected.

If any Dash.jl user is reading this, it would be nice to have some of you try out this branch in your dev environment

julia> import Pkg
julia> Pkg.add(url="https://github.com/plotly/Dash.jl", rev="bring-gen_resources-back-to-life")

thank you !!

@etpinard etpinard mentioned this pull request Jun 28, 2023
Merged
@etpinard
Copy link
Collaborator Author

Now in #212 - closing.

@etpinard etpinard closed this Jun 28, 2023
@etpinard etpinard deleted the bring-gen_resources-back-to-life branch June 28, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
documentation Improvements or additions to documentation priority Dash.jl tasks that should be completed prior to non-priority items tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@etpinard @alexcjohnson