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

4.18 unexpectedly introduces Rust into build requirements #1117

Closed
gmr opened this issue Jul 6, 2023 · 7 comments
Closed

4.18 unexpectedly introduces Rust into build requirements #1117

gmr opened this issue Jul 6, 2023 · 7 comments

Comments

@gmr
Copy link

gmr commented Jul 6, 2023

Introducing rpds-py as a dependency in 4.18 broke builds due to the new Rust requirement for using jsonschema.

IMO this type of change should be a major version change and not a minor one. ie jsonschema should have gone to 5.0.
@Julian
Copy link
Member

Julian commented Jul 6, 2023

See #1114 and crate-py/rpds#6 -- if you're on some other platform which now still does not have rpds-py wheels, a PR or issue is welcome.

IMO this type of change should be a major version change and not a minor one. ie jsonschema should have gone to 5.0.

Your opinion is noted but I (obviously) disagree, this is simply a build time change, not a publicly facing one, and you should be pinning your dependencies.

If you want some previous precedent/context, have a look at (the note at the bottom of) pyca/cryptography#5771

@Julian Julian closed this as not planned Won't fix, can't repro, duplicate, stale Jul 6, 2023
@gmr
Copy link
Author

gmr commented Jul 6, 2023 •
edited
Loading

We generally do loose pins so that fixes to packages get included, but breaking changes don't break things. Am well aware of the cryptography mess.

@darioackermann
Copy link

darioackermann commented Jul 23, 2023 •
edited
Loading

Users using any AWS Lambda resources now need to pin jsonschema down: aws/aws-cdk#26300

@darioackermann darioackermann mentioned this issue Jul 23, 2023
Closed
DnPlas added a commit to canonical/kubeflow-dashboard-operator that referenced this issue Jul 24, 2023
@DnPlas
build: install jsonschema from binary instead of building from src
6598fc5 
Because some dependencies of jsonschema require the rust toolchain (see 1 and 2),
building the charm fails when building and installing some deps of jsonschema.
The workaround to this is to install jsonschema from binary, avoiding unnecesary
toolchains in the charm and reducing the chance of getting affected by third party
dependencies when they are updated.
[1]python-jsonschema/jsonschema#1117
[2] python-jsonschema/jsonschema#1114
@DnPlas DnPlas mentioned this issue Jul 24, 2023
Closed
DnPlas added a commit to canonical/kubeflow-dashboard-operator that referenced this issue Jul 24, 2023
@DnPlas
build: install jsonschema from binary instead of building from src
c340988 
Because some dependencies of jsonschema require the rust toolchain (see 1 and 2), building
the charm fails when building and installing some deps of jsonschema.
The workaround to this is to install jsonschema from binary, avoiding unnecesary toolchains
in the charm and reducing the chance of getting affected by third
party dependencies when they are updated.

Other considerations:
Other options were considered for fixing the issue, like installing rustc in the charm
directly and pinning jsonschema<4.18. The first option seem too invasive as charms will not
use rustc for anything during runtime. The last option restricted us from updating the package
in the event of a bug fix or if a new version is needed.
Installing from binary also has limitations, such as version management, which is now done
in charmcraft.yaml and not in the more common requirement files.

Fixes: #147

[1]python-jsonschema/jsonschema#1117
[2] python-jsonschema/jsonschema#1114
@DnPlas DnPlas mentioned this issue Jul 24, 2023
Merged
DnPlas added a commit to canonical/serialized-data-interface that referenced this issue Jul 25, 2023
@DnPlas
build: pin jsonschema to ^4.17.3 to avoid installing 4.18.x
1191606 
Because some dependencies of jsonschema now require the rust toolchain (see 1 and 2), building
charms that depend on SDI (and therefore pull jsonschema) fail when building because none
of them have rustc available when pip installing the charm's python dependencies.

Reasoning:
jsonschema 4.18.x introduced a dependency on rdps-py which at the same time introduced
a dependency on maturin, a python package that requires the rustc toolchain to be
built and installed from source. jsonschema 4.17.x does not have that dependency.

Links:
[1]python-jsonschema/jsonschema#1117
[2] python-jsonschema/jsonschema#1114

Partially fixes: canonical/bundle-kubeflow#648
@DnPlas DnPlas mentioned this issue Jul 25, 2023
Merged
DnPlas added a commit to canonical/serialized-data-interface that referenced this issue Jul 25, 2023
@DnPlas @addyess @ca-scribner
build: pin jsonschema to >4, <4.18 to avoid installing 4.18.x (#49)
8ab9b71 
* build: pin jsonschema to >4, <4.18 to avoid installing 4.18.x

Because some dependencies of jsonschema now require the rust toolchain (see 1 and 2), building
charms that depend on SDI (and therefore pull jsonschema) fail when building because none
of them have rustc available when pip installing the charm's python dependencies.

Reasoning:
jsonschema 4.18.x introduced a dependency on rdps-py which at the same time introduced
a dependency on maturin, a python package that requires the rustc toolchain to be
built and installed from source. jsonschema <4.18 does not have that dependency.

Links:
[1]python-jsonschema/jsonschema#1117
[2] python-jsonschema/jsonschema#1114

Partially fixes: canonical/bundle-kubeflow#648

Co-authored-by: Adam Dyess <[email protected]>
Co-authored-by: Andrew Scribner <[email protected]>
@DnPlas DnPlas mentioned this issue Jul 25, 2023
Closed
rain0r added a commit to rain0r/stimmungskalender that referenced this issue Aug 7, 2023
@rain0r
Set fixed jsonschema version so Rust is not required.
62d31c7 
python-jsonschema/jsonschema#1117
@be-aws-architect be-aws-architect mentioned this issue Oct 4, 2023
Closed
@luke-jr
Copy link

luke-jr commented Oct 25, 2023

Requiring Rust is a non-starter (it's not really bootstrappable, which is a huge security issue) and pinning dependencies is a bad practice. Please fix this.

@fauxpark
Copy link

if you're on some other platform which now still does not have rpds-py wheels, a PR or issue is welcome.

This is completely impossible on macOS with Homebrew, as it does not use wheels for Python resources. 🤷

@leftiness
Copy link

leftiness commented Mar 15, 2024 •
edited
Loading

This issue did not prevent me from using jsonschema==4.21.1 in a Lambda function. I did not have to pin jsonschema==4.17.3.

context

  • I faced the error with rpds because I built with arm64 and jsonschema==4.21.1, but I ran on x86_64.
  • If you must build on arm64 and run on x86_64, it's possible to work around the error by pinning jsonschema==4.17.3. There may be a better solution. This was suggested elsewhere. lambda: jsonschema import fails with lambda using aws-cdk 2.87.0 aws/aws-cdk#26300
  • You can solve the error by building and running on x86_64.
  • You can solve the error by building and running on arm64. This is the solution that I chose. For example, use build image public.ecr.aws/sam/build-python3.10:1-arm64.
  • It might be relevant that I'm building on a Mac using Docker Desktop v4.28.0.

links

error

[ERROR] Runtime.ImportModuleError: Unable to import module 'index': No module named 'rpds.rpds'

docker images

public.ecr.aws/sam/build-python3.10   latest-x86_64   dbf510ea19fd   47 hours ago    1.57GB
public.ecr.aws/sam/build-python3.10   latest          d6a7783541f0   47 hours ago    1.58GB
public.ecr.aws/sam/build-python3.10   latest-arm64    d6a7783541f0   47 hours ago    1.58GB

@leftiness
Copy link

You can also solve the error by setting necessary flags for pip install. For example, I have a Lambda function with Python 3.10 runtime and arm64 architecture. I built it using alpine:3.

example flags

--only-binary=:all:
--platform manylinux_2_17_aarch64
--python-version 3.10

log

Collecting rpds-py>=0.7.1
  Downloading rpds_py-0.18.0-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl (1.1 MB)

links

@Jan-Heussner Jan-Heussner mentioned this issue Sep 25, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@gmr @Julian @luke-jr @fauxpark @leftiness @darioackermann