Use Python APIs for linter dependencies

[FrankPortman]

Although thrift-pyi currently has pinned dependencies against autoflake and black, calling a CLI subprocess is not guaranteed to actually use those specific dependent packages in lieu of other system installed libraries. This is especially apparent in a Bazel monorepo where commands are executed in quarantined sandbox environments. I'm not sure if virtualenv works ok with the current setup, but I think using the internal APIs may be safer in general, but I am curious to hear your opinions.

[FrankPortman]

It seems like calling the functions internally is maybe not respecting some specific black configurations that have been set

[FrankPortman]

@unmade a different workaround that leverages all the nice parts of the CLI tools would be something like:

def lint(output_dir: Path) -> None:
    subprocess.check_call([f"{sys.executable()} -m autoflake -i -r {output_dir.joinpath('*')}"], shell=True)
    subprocess.check_call(
        ["{sys.executable()} -m black", "--pyi", "--quiet", *list(output_dir.glob("*.pyi"))]
    )

what do you think about that approach?

[FrankPortman]

Here is a branch with the other approach. Passes all tests and lint: master...FrankPortman:thrift-pyi:executable

[unmade]

Hey @FrankPortman

Thanks

In general I agree with you - using black/autoflake python api is a better option than calling it via subprocess. I don't really remember now why I opted out for using subprocess, maybe because these tools didn't provide any python API at the time.

Looking at the current changes everything seems good, but please add poetry-installer-error-***.log files to your local .gitignore. I see some jobs has failed, if you need some help with that, please let me know

[FrankPortman]

Thanks for responding. After playing around with this a bit more, I am a bit nervous to use the internal APIs because they do not trivially respect whatever configuration is being set elsewhere - thus they will not be backwards compatible. E.G. in my current code it keeps trying to add an extra new line character after test imports, but the current master test suite run in a virtualenv does NOT require that new line. So there is some configuration somewhere that the internal APIs are ignoring.

I think this approach may be a happy middle ground where it still uses subprocess (with the drawbacks we agree on), but at least is using the current Python process to find the requisite libraries (via sys.executable). WDYT?

master...FrankPortman:thrift-pyi:executable

[FrankPortman]

Hiya, just wanted to bump this up to see your thoughts on the other approach from my last message. Appreciate your time on this!

[unmade]

I'm really sorry it took me so long to reply to you!

I think the approach with sys.executable totally makes sense and I'll be happy to merge it.

One minor note - according to the docs sys.executable can be either empty string or None. I'm not sure when this can be the case, but I think it's worth to add a check and cast None to empty string

[FrankPortman]

Thanks for the reply @unmade ! Let me close this in favor of #46