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.

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

remove top pin for jsonschema #51

Closed
wants to merge 1 commit into from
Closed

Conversation

bollwyvl
Copy link
Contributor

Hi! Thanks for hologram!

This removes the top pin for jsonschema. Under test, this passes with ~90% coverage over on conda-forge/hologram-feedstock#4 where this pin is becoming problematic for downstreams.

See also #50.

@dnascimento
Copy link

Hi! Could this be merged please? It causes issues with dbt-core

@sandervandorsten
Copy link

sandervandorsten commented Feb 20, 2023

Also curious if this can be merged.

@dnascimento @bollwyvl did you guys find a workaround?

i'm currently trying to install dbt-core inside Amazon MWAA with apache-airflow==2.4.3 using python 3.10. I'm currently stuck on the hologram induced dependency conflict of jsonschema as well (need version jsonschema==4.17.0 based on airflows constraint file in my setup)

@bollwyvl
Copy link
Contributor Author

find a workaround?

Yerp: as mentioned, on conda-forge, we patch the dependency, and are already shipping it. As mentioned, we run the full test suite with coverage, so I'm quite confident it's a "safe" patch.

When this repo finally makes a canonical release with the fix, the conda-forge build will break when the bot PR finds it, we'll remove the patch, and forget this happened.

@sandervandorsten
Copy link

hmm okay, that sounds good! Could you help me out how to use the conda-forge version in my installation? I've never worked with that.

because of my CI/CD requirements I need to specify my dependencies in the requirements.txt file. this looks like so

requirements.txt

--constraint https://raw.githubusercontent.com/apache/airflow/constraints-2.4.3/constraints-3.10.txt
... [other dependencies] 
dbt-core=1.4.1
... [other dependencies]

the dbt-core version uses hologram=0.0.15, which refers to the jsonschema version.
should i then do something like this?

--constraint https://raw.githubusercontent.com/apache/airflow/constraints-2.4.3/constraints-3.10.txt
... [other dependencies] 
hologram==0.0.15 --index-url=[conda-forge/hologram]
dbt-core=1.4.1
... [other dependencies]

@bollwyvl
Copy link
Contributor Author

pip (or whatever .whl-based tool you're using) is entirely unable to install conda-forge-built packages.

The reverse is not true, and tools like conda-lock can help bridge the gap between the two.

If that's not an option, you could be to take the patches applied on the conda-forge repo, and build your own wheels. If that is incompatible with your workflow, and relying on "stuff you find on pypi" as being "the truth," is more important than "it works," then maybe that's a problem with your workflow.

The owners of this repo know this is causing problems for end users: if you are a paying dbt customer, you could escalate to this with them, but otherwise, we (conda-forge) as a downstream have done all we can.

@sandervandorsten
Copy link

sandervandorsten commented Feb 20, 2023

If that's not an option, you could be to take the patches applied on the conda-forge repo, and build your own wheels.

I was already thinking along that axis, it makes sense. We're using a private (AWS CodeArtifact) repository manager, that should work :)

The owners of this repo know this is causing problems for end users: if you are a paying dbt customer, you could escalate to this with them, but otherwise, we (conda-forge) as a downstream have done all we can.

I'll try to get in contact with dbt via our company's partnership, not sure yet if that provides any leverage but maybe that provides a way to escalate, good suggestion!

Thanks!

@gshank
Copy link
Contributor

gshank commented Mar 24, 2023

Already merged this change. Closing.

@gshank gshank closed this Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loosen dependency restriction to accept jsonschema 4.0 or better still, remove the upper bound
4 participants