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

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

Open
dintorf opened this issue Jun 3, 2024 · 7 comments
Assignees
Labels
bug bug community community Effort - Low Effort - Low Frequency - EveryTime Frequency - EveryTime Priority - Later Priority - Later Reach - Some Reach - Some Severity - S3 Severity - S3

Comments

@dintorf
Copy link

dintorf commented Jun 3, 2024

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
Copy link
Contributor

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
Copy link
Author

dintorf commented Jun 5, 2024

Awesome! Looking forward to it :)

@katarinasupe
Copy link
Contributor

Hi @dintorf, did you receive my email?

@dintorf
Copy link
Author

dintorf commented Jun 7, 2024

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
Copy link
Contributor

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

@dintorf
Copy link
Author

dintorf commented Jun 12, 2024

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

@katarinasupe
Copy link
Contributor

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

@hal-eisen-MG hal-eisen-MG added the Priority - Later Priority - Later label Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bug community community Effort - Low Effort - Low Frequency - EveryTime Frequency - EveryTime Priority - Later Priority - Later Reach - Some Reach - Some Severity - S3 Severity - S3
Projects
Development

No branches or pull requests

5 participants