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

Add a mechanism for avoiding hashing sources on daemon start #6389

Merged
merged 7 commits into from Feb 15, 2019

Conversation

Projects
None yet
3 participants
@msullivan
Copy link
Collaborator

commented Feb 13, 2019

This has two parts:

  • Add an argument to dmypy status (--fswatcher-dump-file JSONFILE) that tells the daemon to
    write out the state of fswatcher to a file.
  • Allow mypy/dmypy to use this information to avoid hashing source
    files if the mtime in the cache disagrees with the disk but
    the old fswatcher info agrees. (--quickstart-file JSONFILE)
Add a mechanism for avoiding hashing sources on daemon start
This has two parts:
 * Add an argument to dmypy status that tells the daemon to
   write out the state of fswatcher to a file.
 * Allow mypy/dmypy to use this information to avoid hashing source
   files if the mtime in the cache disagrees with the disk but
   the old fswatcher info agrees.

@msullivan msullivan requested review from JukkaL and ilevkivskyi Feb 13, 2019

Show resolved Hide resolved test-data/unit/daemon.test Outdated
def read_quickstart_file(options: Options) -> Optional[Dict[str, Tuple[float, int, str]]]:
quickstart = None # type: Optional[Dict[str, Tuple[float, int, str]]]
if options.quickstart_file:
# This is very "best effort". If the file is missing or malformed,

This comment has been minimized.

Copy link
@ethanhs

ethanhs Feb 13, 2019

Collaborator

I feel like we should at least warn on a bad file. That will also allow our tests to catch bugs.

This comment has been minimized.

Copy link
@gvanrossum

gvanrossum Feb 13, 2019

Member

Hm, I actually agree: warn but continue.

This comment has been minimized.

Copy link
@msullivan

msullivan Feb 14, 2019

Author Collaborator

mypy isn't very good at warning, though. We can label something as a warning, but it will still cause an error exit code.

We could just do a print, but then that will end up in the log file for the daemon (which is maybe fine?).

This comment has been minimized.

Copy link
@gvanrossum

gvanrossum Feb 14, 2019

Member

sys.stderr.write("warning: Blah\n") sounds acceptable, assuming that will be visible when we run tests?

This comment has been minimized.

Copy link
@ethanhs

ethanhs Feb 14, 2019

Collaborator

Yeah the daemon tests capture stderr so it should be fine.

This comment has been minimized.

Copy link
@msullivan

msullivan Feb 14, 2019

Author Collaborator

It can be made visible by having the test dump the log file

Update test-data/unit/daemon.test
Co-Authored-By: msullivan <sully@msully.net>
@gvanrossum
Copy link
Member

left a comment

This looks good, though I had to read the comment you added to validate_meta() three times before I got it.

Show resolved Hide resolved mypy/build.py Outdated
def read_quickstart_file(options: Options) -> Optional[Dict[str, Tuple[float, int, str]]]:
quickstart = None # type: Optional[Dict[str, Tuple[float, int, str]]]
if options.quickstart_file:
# This is very "best effort". If the file is missing or malformed,

This comment has been minimized.

Copy link
@gvanrossum

gvanrossum Feb 13, 2019

Member

Hm, I actually agree: warn but continue.

Show resolved Hide resolved test-data/unit/daemon.test
@msullivan

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 14, 2019

This looks good, though I had to read the comment you added to validate_meta() three times before I got it.

Do you have any suggestions on language revision to that comment that might make it more clear?

@gvanrossum

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

Do you have any suggestions on language revision to that comment that might make it more clear?

Alas, not -- it's just complicated.

@msullivan

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 14, 2019

@gvanrossum Anything else, or is this good?

@gvanrossum
Copy link
Member

left a comment

LGTM.

@msullivan msullivan merged commit 86515b5 into master Feb 15, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@msullivan msullivan deleted the load_graph branch Feb 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.