memgraph / mage

MAGE - Memgraph Advanced Graph Extensions :crystal_ball:
Apache License 2.0
238 stars 23 forks source link

[BUG] build_and_copy_rust_modules directory loop is broken and only catches the first module in the list #477

Open dintorf opened 1 month ago

dintorf commented 1 month ago

Describe the bug The build_and_copy_rust_modules function in the setup script does not properly loop through the module directory. The filter function is a generator and is executed on the fly, so since directory changes occur inside the loop the os.path.isdir(f) part of the loop breaks.

To Reproduce Steps to reproduce the behavior:

  1. Add a new rust module in the rust directory that falls after the rust_example module alphabetically
  2. Build and run the docker image
  3. Shell into the running docker container
  4. Execute the setup script with python3 setup build --lang rust -p /usr/lib/memgraph/query_modules
  5. Observe that the new package is not built and copied to query_modules

Expected behavior It's expected that all rust modules in the directory are built and copied to the output path except for rsmgp-sys.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context This can be fixed by changing the for loop in the build_and_copy_rust_modules function to the following:

for project in filter(
    lambda f: os.path.isdir(os.path.join(RS_DIRECTORY, f)) and f != "rsmgp-sys",
    os.listdir(RS_DIRECTORY),
)
katarinasupe commented 4 weeks ago

Hi @dintorf! Thank you so much for detecting this issue and fixing it. As a thank you, I'll reach out to you via email to ask for delivery info so I can send you Memgraph swag 🚀

dintorf commented 4 weeks ago

Awesome! Looking forward to it :)

katarinasupe commented 3 weeks ago

Hi @dintorf, did you receive my email?

dintorf commented 3 weeks ago

Hi @katarinasupe, I did! I sent a reply. Let me know if you didn’t get it and I’ll send it from a different email address.

katarinasupe commented 3 weeks ago

Not sure why, but I didn't get the reply. @dintorf please send it for a different email address 🙏

dintorf commented 3 weeks ago

Hi @katarinasupe, I sent you another email last week. Did you happen to receive that one?

katarinasupe commented 3 weeks ago

Hi @dintorf, I received it! I will respond asap, it's been a busy week 😄