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

More sensible error message for runscript #955

Closed
Deleetdk opened this issue Sep 5, 2016 · 7 comments
Closed

More sensible error message for runscript #955

Deleetdk opened this issue Sep 5, 2016 · 7 comments

Comments

@Deleetdk
Copy link

Deleetdk commented Sep 5, 2016

I wasted a lot of time discovering that runscript gives the same error message for when there is an import error in the script and when it cannot find the script at all. Please add a check for whether the file exists at all first, and throw a distinct error if it cannot find any such script.

StackOverflow example.

@kevgathuku
Copy link
Contributor

Hey @Deleetdk, I have just been looking through this and I would like to tackle it. Sorry for the bad experience you had to go through to discover this bug.

After looking into it, I think the error message for a non-existent script is correct.
i.e. No (valid) module for script 'test_script' found

I think the error message for when there's an import error in the script is what needs to be fixed.
e.g. In the case where import nonexistrentpackage is at the top of the file.

However, you will notice that when the invalid import is placed inside the run function, the correct error message is raised. For example, see the following code and the error message it raises.

def run():
    import nonexistent
    print("Test")

Exception:

...
ImportError: No module named nonexistent

I think this is what should be raised when there is an invalid import.
What do you think?

@Deleetdk
Copy link
Author

Makes sense. However, I didn't know that I had to put my imports inside run() too. I noticed that the official docs also have imports outside run(). It's not a common coding pattern to do, so it's likely that many others will run into this same problem.

@kevgathuku
Copy link
Contributor

@Deleetdk that's not the recommended way to do it. I just tried it out and noticed it handled the error correctly.
I'll try to see whether I can replicate that kind of error even when the invalid import is at the top level.

kevgathuku added a commit to kevgathuku/django-extensions that referenced this issue Sep 18, 2016
Resolves django-extensions#955
- When a missing import is found in a script, this changes the error
message displayed to indicate that there's a missing import, rather than
giving a generic error message similar to the one raised when the script
does not exist
@trbs
Copy link
Member

trbs commented Sep 25, 2016

The imports inside the run() function should not be necessary. It is a workaround to avoid some of the issues with unclear messages.

Thanks to @kevgathuku we have #964 now !
I would like to see us fixing all potential problems with errors in the scripts not just ImportError. We have a similar issue with AttributeError as well.

@trbs
Copy link
Member

trbs commented Sep 25, 2016

Would be cool if we could get test cases as well for this to avoid regressions in the future.

@hhstore
Copy link

hhstore commented Aug 31, 2017

No (valid) module for script 'test_script' found

The error info is confusing.

  • the runscript did not catch the exception.

Reason:

  • sometimes, this error was caused by your codes have bugs.
  • like:
    • your code import a third party package, and the package has bugs,
    • It is difficult for you to find errors caused by the third party package.

How to fix it:

  • so, you need use try ... catch to get the exception.
  • then after you fix the bug, you will find the runscript is ok now.
# runscript file:
def run():
    try:
        from xxx import  xxx    # maybe the third party package has bugs.
        do_xxx()
    except Exception as e:
        print(e)
        raise e
    

@trbs trbs closed this as completed in e85994f Sep 11, 2017
@mikbuch
Copy link

mikbuch commented Jan 26, 2022

Hi guys, I'm not sure wether open a new issue for that, but:

The solution of this issue -- a try-except clause -- doesn't help in the situation in which you have wrong file extension of your script (e.g., just scripts/django-script (no .py extension) instead of scripts/django-script.py), and you are running the proper command:

python manage.py runscript django-script

Then you are still getting:

No (valid) module for script 'django-script' found

It should either just run, or you should get something like:

Wrong extension of your scripts. It should have a ".py" extension.

Similar situation is when you have your script under:

scripts/django-script.py

and you are trying to run:

python manage.py runscript scripts/django-script

From the usability point of view, it should either just run, or it should inform the user that, e.g.:

You should specify just the name of the script (without the extension), instead of the path.

People (including me) are still having problems with that, see, e.g.: https://stackoverflow.com/a/67440238/8877692

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

No branches or pull requests

5 participants