fix: python dependency specification

GlareDB / glaredb

GlareDB: An analytics DBMS for distributed data

https://glaredb.com

GNU Affero General Public License v3.0

550 stars 36 forks source link

fix: python dependency specification #2483

Closed tychoish closed 3 months ago

tychoish commented 4 months ago

I expect the environment/optional dependency thing something we should discuss (do we want to force pulling in pandas and polars? Probably dev dependencies are going to lead to some kind of issue...

talagluck commented 4 months ago

I don't think we want to force pulling in polars and I'm not sure about pandas. The more dev dependencies we have the harder this will be to navigate (ran into this a lot at a previous place) and pip's dependency resolver can be kind of heavy, so unless something is strictly necessary for core functionality, it should be optional

tychoish commented 4 months ago

in addition to making things optional dependencies, we should also properly handle when the deps are not installed.

It should error instead of panicking

Totally agree. I think we should probably do that work as part of a seperate ticket just to fix the bug, incase we do a point release early next week.

Do you, @universalmind303 or @talagluck, want to take this over? g

tychoish commented 4 months ago

I spent a couple hours playing with this, and I have something that maybe works:

installs build time dependencies and test/runtime dependencies into different virtualvens that are scoped to the bindings.
installs the extra dependencies (I think we want to have this off by default?
the install succeeds but it has a patchelf error message. I was able to run something locally, but I didn't kick the tires very hard.

I'm not sure how to do the better error messages you're interested in Cory, but that'd definitely be a good thing to add.

tychoish commented 3 months ago

I did a different update to the pytests, and this should just hit the python deps. There was a merge issue for a while, but I've cherry-picked and force pushed this for the moment.

Remaining work should be buildsystem related, I believe.

tychoish commented 3 months ago

@universalmind303 could you take another look.

tychoish commented 3 months ago

@talagluck @universalmind303 can you take another look at this?