python-jsonschema / check-jsonschema

A CLI and set of pre-commit hooks for jsonschema validation with built-in support for GitHub Workflows, Renovate, Azure Pipelines, and more!
https://check-jsonschema.readthedocs.io/en/stable
Other
192 stars 38 forks source link

OverflowError: mktime argument out of range #275

Closed balihb closed 1 year ago

balihb commented 1 year ago

Trying to use the following schema URL:

https://raw.githubusercontent.com/microsoft/AI/master/.ci/listing_config_schema.json

Gives the following error on the 2nd run:

Error: Unexpected Error building schema validator
OverflowError: mktime argument out of range
  in "C:\Users\balihb\.cache\pre-commit\repos2k4wtzx\py_env-python3.11\Lib\site-packages\check_jsonschema\checker.py", line 52
  >>> return self._schema_loader.get_validator(
sirosen commented 1 year ago

Thanks for the bug report!

I wasn't able to reproduce this. In fact, I'm not able to use that schema at all, as it does not appear to be a valid JSON Schema. Checking it with --check-metaschema fails with a long list of errors.

Maybe the schema has changed between the time of your report and my test? I don't have a lot of bandwidth to look into this right now, but if the schema is valid, that would mean something is wrong with the validation currently done under --check-metaschema.

balihb commented 1 year ago

I was trying on Windows. might it be a problem with platform support? check-jsonschema also failing for me on any other chema from github too.

On Fri, Jun 9, 2023, 18:47 Stephen Rosen @.***> wrote:

Thanks for the bug report!

I wasn't able to reproduce this. In fact, I'm not able to use that schema at all, as it does not appear to be a valid JSON Schema. Checking it with --check-metaschema fails with a long list of errors.

Maybe the schema has changed between the time of your report and my test? I don't have a lot of bandwidth to look into this right now, but if the schema is valid, that would mean something is wrong with the validation currently done under --check-metaschema.

— Reply to this email directly, view it on GitHub https://github.com/python-jsonschema/check-jsonschema/issues/275#issuecomment-1584877427, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAV4ZGG65A6TM2XTS3C7LUTXKNHTNANCNFSM6AAAAAAY7MLVTA . You are receiving this because you authored the thread.Message ID: @.***>

sirosen commented 1 year ago

I was trying on Windows. might it be a problem with platform support?

It shouldn't be, but also it shouldn't fail at all so I'm remaining open-minded. :wink:

Could you post the results of running one of the failing commands with --traceback-mode full? My current suspicion is that something is amiss with the handling of Last-Modified times for file downloads, but it's not something I've seen before, so I need a bit more info to track it down.

balihb commented 1 year ago

I will give it a try on linux too. I haven't run it in debug, that will probably be enough to find the problem. expect a PR soon enough.

On Fri, Jun 9, 2023, 21:31 Stephen Rosen @.***> wrote:

I was trying on Windows. might it be a problem with platform support?

It shouldn't be, but also it shouldn't fail at all so I'm remaining open-minded. 😉

Could you post the results of running one of the failing commands with --traceback-mode full? My current suspicion is that something is amiss with the handling of Last-Modified times for file downloads, but it's not something I've seen before, so I need a bit more info to track it down.

— Reply to this email directly, view it on GitHub https://github.com/python-jsonschema/check-jsonschema/issues/275#issuecomment-1585042257, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAV4ZGETVHRGCEZCEPK3K5TXKN2XPANCNFSM6AAAAAAY7MLVTA . You are receiving this because you authored the thread.Message ID: @.***>

kurtmckee commented 1 year ago

@balihb I have seen OverflowError pop up with datetime operations on Windows when it's a 32-bit version of Python. Would you run the following to help identify whether this is 32-bit or 64-bit Python?

python -c "import sys; print(sys.version)"

That should help focus the troubleshooting effort, by either identifying -- or eliminating -- one possible cause for that OverflowError. Thanks!

balihb commented 1 year ago

I'm not in front of a machine, but I am fairly sure I'm using 64 python. but will check tomorrow.

On Fri, Jun 9, 2023, 23:26 Kurt McKee @.***> wrote:

@balihb https://github.com/balihb I have seen OverflowError pop up with datetime operations on Windows when it's a 32-bit version of Python. Would you run the following to help identify whether this is 32-bit or 64-bit Python?

python -c "import sys; print(sys.version)"

That should help focus the troubleshooting effort, by either identifying -- or eliminating -- one possible cause for that OverflowError. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/python-jsonschema/check-jsonschema/issues/275#issuecomment-1585144360, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAV4ZGCXF6SQJWK7H2FBBY3XKOIH7ANCNFSM6AAAAAAY7MLVTA . You are receiving this because you were mentioned.Message ID: @.***>

kurtmckee commented 1 year ago

Thanks @balihb, and have a great weekend!

balihb commented 1 year ago

ok, I have two solution:

    def _lastmod_from_response(self, response: requests.Response) -> float:
        if "last-modified" in response.headers:
            return time.mktime(
                time.strptime(
                    response.headers.get("last-modified"),
                    self._LASTMOD_FMT,
                )
            )
        else:
            return time.mktime(time.gmtime(2*86400)) - 2*86400

but I think this should be enough:

    def _lastmod_from_response(self, response: requests.Response) -> float:
        if "last-modified" in response.headers:
            return time.mktime(
                time.strptime(
                    response.headers.get("last-modified"),
                    self._LASTMOD_FMT,
                )
            )
        else:
            return 0
balihb commented 1 year ago

similar issue I've found: https://github.com/neo4j/neo4j-python-driver/issues/302 also from the time doc: The earliest date for which it can generate a time is platform-dependent.

balihb commented 1 year ago

using dateutils might also be a good option: https://dateutil.readthedocs.io/en/stable/parser.html#dateutil.parser.parse https://stackoverflow.com/a/52157795

sirosen commented 1 year ago

Thanks for running this issue down and sharing your research!

@kurtmckee and I chatted about this a little bit, and I'm going to take a slightly different approach to fixing this, using try-except to handle a few different cases.

I'll post a PR shortly, so you'll be able to see the fix with tests, but the additional scenarios we wanted to handle include:

@balihb, I'll make sure to credit you for your work in the changelog! :+1:

sirosen commented 1 year ago

This should be fixed in v0.23.2 which I just released.

Please let me know if you still see this problem or run into other issues!