Skip to content

Conversation

@amotl
Copy link
Contributor

@amotl amotl commented Apr 23, 2025

Problem

With Marshmallow 4, just released, Superset 4 trips like:

TypeError: Field.__init__() got an unexpected keyword argument 'minLength'

Solution

Pin dependency to marshmallow<4.

References

Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've completed my review and didn't find any issues.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

@rusackas rusackas added the review:checkpoint Last PR reviewed during the daily review standup label Apr 23, 2025
@michael-s-molina michael-s-molina removed the review:checkpoint Last PR reviewed during the daily review standup label Apr 25, 2025
"jsonpath-ng>=1.6.1, <2",
"Mako>=1.2.2",
"markdown>=3.0",
"marshmallow<4",
Copy link
Member

@mistercrunch mistercrunch Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's always put a comment above when setting an upper bound that's not based on pure semver assumptions, something like "marshmallow>=4 has shown issues related to 'minLength'", ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I didn't see such comments on other spots, but I've just added it, effectively linking to the corresponding issue on GitHub.

@amotl amotl force-pushed the fix-marshmallow-4.1 branch from 7150804 to ba6e084 Compare April 28, 2025 17:00
@github-actions github-actions bot added i18n Namespace | Anything related to localization risk:db-migration PRs that require a DB migration i18n:spanish Translation related to Spanish language i18n:italian Translation related to Italian language i18n:french Translation related to French language i18n:chinese Translation related to Chinese language i18n:japanese Translation related to Japanese language labels Apr 28, 2025
Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - curious about your internal release process that lead to that issue, do you run your own uv pip compile --upgrade process internally? Or maybe you have your own snyk/dependabot-type setup in place and upgrade certain things?

In any case, would be great to centralize the dependency management and upgrades in the main repo as much as possible. Could certainly use help with the process if you wanted to get involved in dep management.

@mistercrunch mistercrunch merged commit 547c28f into apache:4.1 Apr 28, 2025
27 of 28 checks passed
@amotl
Copy link
Contributor Author

amotl commented Apr 28, 2025

@mistercrunch: Thanks for merging.

curious about your internal release process that lead to that issue

You can inspect the testing rig which flagged this regression through its nightly CI runs over here. HTH, otherwise feel free to ask.

@amotl
Copy link
Contributor Author

amotl commented Jun 11, 2025

Hi again. This still needs a release, in order to resolve the issue completely for average downstream users.

/cc @mistercrunch, @rusackas

@amotl
Copy link
Contributor Author

amotl commented Jun 11, 2025

In any case, would be great to centralize the dependency management and upgrades in the main repo as much as possible. Could certainly use help with the process if you wanted to get involved in dep management.

It looks like this might still be an unresolved issue? Maybe I can free up some time for that, as I guess it might be a one-time action? On the other hand, I strongly believe Superset has its intriciacies, so I certainly would need to learn a significant amount of your current philosophy and about your most prominent pain points in order to possibly improve things thereafter, if at all.

@mistercrunch
Copy link
Member

mistercrunch commented Jun 12, 2025

This could be pull in our various active LTS branches though release managers are already pretty busy with everything and LTS-done-right is difficult, and I believe it's an area where only committers can do certain things (people without write access to the repo simply cannot push a release through for good reasons). Tagging the folks that have been doing release management over the past few releases here: @michael-s-molina @sadpandajoe

On your side though, wondering if your build process can build upon Superset's pinned dependencies. Say if you use pip compile or uv pip compile for you project, you may be able to add a constraint on our pinned deps in your build process. Notice the -c in this script that compiles our deps here -> https://github.com/apache/superset/blob/master/scripts/uv-pip-compile.sh#L28 as you may be able to do something similar. Now it may conflict with with core app's dependencies, but at least you would integrate our pinned requirements along yours, and have to resolve them pre-build.

Superset is/was designed to be in a fairly standalone mode, meaning we're assuming your deploying the Superset app, not another app that happens to be integrating Superset. The way I've seen Superset be a dependency of a wider project is typically by creating a wrapper/shell around it while bringing the Superset repo as a git submodule inside your project or repo. Typically the wrapper will have CI/cloud-provisioning things like terraform/docker and whatever is needed to integrate Superset in your organization's typical build process and requirements. Clearly there's a lot of different constraints for different organizations (everyone has different reqs on how they deploy services internally), and it's been difficult to share the various building blocks and reusable components that caters to such a wide variety of ways of doing things ... Even things like the Helm chart or Dockerifle often needs to be forked for things to work. Similar with terraform, usually there are more things in a typical terraform package that are related to your org's specificities as there are things related to "how Superset should be deployed".

Anyhow, open to helping out around it and offering the buildings blocks that are useful to most.

One thing I'd like to see if "reference implementations" OUTSIDE our repos showing various ways around how this can be done in different places...

@sadpandajoe sadpandajoe added the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Jun 12, 2025
@sadpandajoe
Copy link
Member

I think I had already cherried this as part of 4.1.3rc3, so it should be there already.

@michael-s-molina michael-s-molina added the v5.0 Label added by the release manager to track PRs to be included in the 5.0 branch label Jun 12, 2025
@zhangyijun
Copy link

Lastest 5.0.0 version release, this bug is still exists. pip install will install marshmallow-4.0.0. CAN NOT work anything untill mannually run pip install -U marshmallow==3.26.1.

@betodealmeida
Copy link
Member

Pinned it here: #33978

@betodealmeida betodealmeida mentioned this pull request Jun 30, 2025
9 tasks
@michael-s-molina michael-s-molina removed the v5.0 Label added by the release manager to track PRs to be included in the 5.0 branch label Jul 21, 2025
landryb added a commit to georchestra/superset-core that referenced this pull request Aug 20, 2025
cyber-jessie added a commit to CybercentreCanada/superset that referenced this pull request Jan 8, 2026
* chore: bump base image in Dockerfile with `ARG PY_VER=3.11.11-slim-bookworm` (apache#32780)

* chore: Revert "chore: bump base image in Dockerfile with `ARG PY_VER=3.11.11-slim-bookworm`" (apache#32782)

* fix(chart data): removing query from /chart/data payload when accessing as guest user (apache#30858)

(cherry picked from commit dd39138)

* fix: upgrade to 3.11.11-slim-bookworm to address critical vulnerabilities (apache#32240)

(cherry picked from commit ad05732)

* fix(model/helper): represent RLS filter clause in proper textual SQL string (apache#32406)

Signed-off-by: hainenber <[email protected]>
(cherry picked from commit ff0529c)

* fix: Log table retention policy (apache#32572)

(cherry picked from commit 89b6d7f)

* fix(welcome): perf on distinct recent activities (apache#32608)

(cherry picked from commit 832e028)

* fix(log): Update recent_activity by event name (apache#32681)

(cherry picked from commit 449f51a)

* fix: Signature of Celery pruner jobs (apache#32699)

(cherry picked from commit df06bdf)

* fix(logging): missing path in event data (apache#32708)

(cherry picked from commit cd5a943)

* fix(fe/dashboard-list): display modifier info for `Last modified` data (apache#32035)

Signed-off-by: hainenber <[email protected]>
(cherry picked from commit 88cf2d5)

* fix: make packages PEP 625 compliant (apache#32866)

Co-authored-by: Michael S. Molina <[email protected]>
(cherry picked from commit 6e02d19)

* all cccs changes

* fix: Downgrade to marshmallow<4 (apache#33216)

* fix(log): store navigation path to get correct logging path (apache#32795)

(cherry picked from commit 4a70065)

* fix(pivot-table): Revert "fix(Pivot Table): Fix column width to respect currency config (apache#31414)" (apache#32968)

(cherry picked from commit a36e636)

* fix: improve error type on parse error (apache#33048)

(cherry picked from commit ed0cd5e)

* fix(plugin-chart-echarts): remove erroneous upper bound value (apache#32473)

(cherry picked from commit 5766c36)

* fix(pinot): revert join and subquery flags (apache#32382)

(cherry picked from commit 822d72c)

* fix: loading examples from raw.githubusercontent.com fails with 429 errors (apache#33354)

(cherry picked from commit f045a73)

* chore: creating 4.1.3rc1 change log and updating frontend json

(cherry picked from commit 72cf9b6)

* chore(🦾): bump python sqlglot 26.1.3 -> 26.11.1 (apache#32745)

Co-authored-by: GitHub Action <[email protected]>
(cherry picked from commit 66c1a6a)

* chore(🦾): bump python h11 0.14.0 -> 0.16.0 (apache#33339)

Co-authored-by: GitHub Action <[email protected]>
(cherry picked from commit 8252686)

* docs: CVEs fixed on 4.1.2 (apache#33435)

(cherry picked from commit 8a8fb49)

* feat(api): Added uuid to list api calls (apache#32414)

(cherry picked from commit 8decc9e)

* fix(table-chart): time shift is not working (apache#33425)

(cherry picked from commit dc44748)

* fix(Sqllab):  Autocomplete got stuck in UI when open it too fast (apache#33522)

(cherry picked from commit b4e2406)

* chore: update Dockerfile - Upgrade to 3.11.12 (apache#33612)

(cherry picked from commit f0b6e87)

* chore: updating 4.1.3rc2 change log

* Select all Drag and Drop (#546)

* add a select all button for the dnd select

* remove cypress

* chore(deps): bump cryptography from 43.0.3 to 44.0.1 (apache#32236)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit fa09d81)

* fix: Adds missing __init__ file to commands/logs (apache#33059)

(cherry picked from commit c1159c5)

* fix: Saved queries list break if one query can't be parsed (apache#34289)

(cherry picked from commit 1e5a4e9)

* chore: Adds 4.1.4RC1 data to CHANGELOG.md and UPDATING.md

* tag bump for select all drag and drop

* Fix package-lock.json

* Add db migration, bump Docker image base

* gevent for gunicorn

* remove threads and make worker-connections configurable

* Fix package-lock.json

* tag bump for cccs build

* Remove CCCS Dataset Explorer (#550)

* tag bump for CCCS build

---------

Signed-off-by: hainenber <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: gpchandran <[email protected]>
Co-authored-by: Joe Li <[email protected]>
Co-authored-by: Jack <[email protected]>
Co-authored-by: Đỗ Trọng Hải <[email protected]>
Co-authored-by: Michael S. Molina <[email protected]>
Co-authored-by: JUST.in DO IT <[email protected]>
Co-authored-by: Michael S. Molina <[email protected]>
Co-authored-by: Andreas Motl <[email protected]>
Co-authored-by: Ville Brofeldt <[email protected]>
Co-authored-by: Yuri <[email protected]>
Co-authored-by: Maxime Beauchemin <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: sha174n <[email protected]>
Co-authored-by: Paul Rhodes <[email protected]>
Co-authored-by: Rafael Benitez <[email protected]>
Co-authored-by: cccs-RyanK <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: cyber-jessie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants