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

fix problem with non-exported function #603

Conversation

maxpohlmann
Copy link
Contributor

We had a problem where Quantum could only schedule a job right after executing mix clean, but with every call to mix after that, our configured jobs were not scheduled, because function_exported?/3 returned false, even though the function still existed, but apparently was not yet exported at the time the Quantum code executed. This PR fixes this by calling Code.ensure_loaded?/1 before function_exported?/3, which actively tries to load the module before returning true if this was successful or false otherwise (e.g. if the module does not exist).

lib/quantum.ex Outdated
@@ -343,7 +343,7 @@ defmodule Quantum do

defp invalid_job_task?(%Job{task: {m, f, args}})
when is_atom(m) and is_atom(f) and is_list(args),
do: not function_exported?(m, f, length(args))
do: not (Code.ensure_loaded?(m) && function_exported?(m, f, length(args)))
Copy link
Member

@maennchen maennchen Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is quite hard to read. Can we switch it to something like this?

if Code.ensure_loaded?(m),
  do: not(function_exported?(m, f, length(args))),
  else: true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, it's definitely more readable that way. Also, thank you for reacting so quickly! 🙏

@maennchen maennchen self-assigned this Feb 29, 2024
@maennchen maennchen added the bug label Feb 29, 2024
@coveralls
Copy link

coveralls commented Feb 29, 2024

Pull Request Test Coverage Report for Build 7047ec21deddb8daa1a64b6a04869c7a6d3fa56b-PR-603

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 86.453%

Totals Coverage Status
Change from base Build 0c34ca8dc6937f148b6e272dfd621251e6da8765: 0.03%
Covered Lines: 351
Relevant Lines: 406

💛 - Coveralls

@maennchen maennchen enabled auto-merge (squash) February 29, 2024 09:52
@maennchen maennchen merged commit 95d742e into quantum-elixir:main Feb 29, 2024
14 checks passed
@maxpohlmann maxpohlmann deleted the fix_problem_with_non-exported_function branch February 29, 2024 10:05
@maxpohlmann
Copy link
Contributor Author

Thank you! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants