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

Revisit macos #2

Merged
merged 32 commits into from
Jul 14, 2023
Merged

Revisit macos #2

merged 32 commits into from
Jul 14, 2023

Conversation

mhaley-tignis
Copy link
Owner

No description provided.

@mhaley-tignis mhaley-tignis self-assigned this Jul 11, 2023
@wbarnha
Copy link

wbarnha commented Jul 12, 2023

It's been a while since I attempted this, so I've forgotten some things. I see you're trying to build for v6.29.5, I wonder how the build would work for v6.14.6?

@mhaley-tignis
Copy link
Owner Author

v6.14.6

Is v6.14.6 a requirement or does any version of v6 work? I don't mind trying it out, I just found the error from v6.29.5 to be easier to understand.

Also, you mentioned rocksdict previously. Is this the library that faust will use moving forward?

@wbarnha
Copy link

wbarnha commented Jul 13, 2023

v6.14.6

Is v6.14.6 a requirement or does any version of v6 work? I don't mind trying it out, I just found the error from v6.29.5 to be easier to understand.

Weird that the older version of RocksDB doesn't work. Maybe v6.29.3 should be for macOS and v6.14.6 is for Linux.

Also, you mentioned rocksdict previously. Is this the library that faust will use moving forward?

Yes, since rocksdict is 1) actively maintained, 2) the wheels are significantly smaller, 3) much more efficient than python-rocksdb, and 4) supports many architectures and versions of Python. python-rocksdb has unfortunately become a disorganized mess, and the wheel files being 66 MB in size each are an unfortunate compromise to make python-rocksdb more accessible to people at the cost of it being oversized and undermaintained.

@mhaley-tignis
Copy link
Owner Author

Also, you mentioned rocksdict previously. Is this the library that faust will use moving forward?

Yes, since rocksdict is 1) actively maintained, 2) the wheels are significantly smaller, 3) much more efficient than python-rocksdb, and 4) supports many architectures and versions of Python. python-rocksdb has unfortunately become a disorganized mess, and the wheel files being 66 MB in size each are an unfortunate compromise to make python-rocksdb more accessible to people at the cost of it being oversized and undermaintained.

I think we are fine to switch over to rocksdict then. I moved just the macos build back to v6.29.5. If the build passes yay!, if not, I will leave this up for the next person that may need it. Should we add a banner to this repository and explain that it is recommended to move to using rocksdict instead (upstream faust should likely be updated to state this too)?

@wbarnha
Copy link

wbarnha commented Jul 13, 2023

Also, you mentioned rocksdict previously. Is this the library that faust will use moving forward?

Yes, since rocksdict is 1) actively maintained, 2) the wheels are significantly smaller, 3) much more efficient than python-rocksdb, and 4) supports many architectures and versions of Python. python-rocksdb has unfortunately become a disorganized mess, and the wheel files being 66 MB in size each are an unfortunate compromise to make python-rocksdb more accessible to people at the cost of it being oversized and undermaintained.

I think we are fine to switch over to rocksdict then. I moved just the macos build back to v6.29.5. If the build passes yay!, if not, I will leave this up for the next person that may need it. Should we add a banner to this repository and explain that it is recommended to move to using rocksdict instead (upstream faust should likely be updated to state this too)?

I did add a note telling people to use rocksdict in my python-rocksdb fork, but it seems to not be getting enough attention. I'd like to archive my project but I'm hesitant to archive anything that people still use.

I also added a note to the README in Faust regarding rocksdict so people can run pip install faust-streaming[rocksdict]. I'm hesitant to update faust-streaming[rocksdb] to install rocksdict because I'm cautious of messing up people's systems whose code is dependent on libraries only compatible with RocksDB 6.

I didn't widely advertise the usage of rocksdict with Faust until I've used it enough to verify its stability, which I can now say with certainty that it's incredibly stable and useful.

I do think that the documentation should be updated in more places to make this note more clear. I just haven't had the free time lately to maintain the project as much as I'd like.

@wbarnha
Copy link

wbarnha commented Jul 13, 2023

I see you disabled the arm64 builds, that could be the culprit for hanging. I think I ran into similar issues in the past.

@mhaley-tignis
Copy link
Owner Author

Looks like build is passing now @wbarnha. Want me to open a PR into https://github.com/faust-streaming/python-rocksdb?

@wbarnha
Copy link

wbarnha commented Jul 13, 2023

Looks like build is passing now @wbarnha. Want me to open a PR into https://github.com/faust-streaming/python-rocksdb?

Yes, please do! Thank you very much for tackling this, I was quite disappointed that I walked away from it but it's nice to see the issue solved now.

@mhaley-tignis mhaley-tignis merged commit d5f757f into master Jul 14, 2023
@mhaley-tignis mhaley-tignis deleted the revisit-macos branch July 14, 2023 12:56
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.

2 participants