Commit Graph

8 Commits

Author SHA1 Message Date
Michael Suo
5caccbe39e [pkg] Catch exceptions where dependency resolution gets invalid imports (#58573)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/58573

Users can create invalid imports, like:
```
HG: in a top-level package
if False:
  from .. import foo
```

Since this code is never executed, it will not cause the module to fail to
load. But our dependency analysis walks every `import` statement in the AST,
and will attempt to resolve the (incorrectly formed) import, throwing an exception.

For posterity, the code that triggered this: https://git.io/JsCgM

Differential Revision: D28543980

Test Plan: Added a unit test

Reviewed By: Chillee

Pulled By: suo

fbshipit-source-id: 03b7e274633945b186500fab6f974973ef8c7c7d
2021-05-19 23:04:21 -07:00
Michael Suo
703f24397b [pkg] simplifications to broken dependency handling (#58572)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/58572

Right now, we have three categories of error (broken, denied, unhandled). This
PR unifies them into a single "error" field in the node, with optional context.
It also generalizes how formatting of the error in PackagingError occurs.

Differential Revision: D28543982

Test Plan: sandcastle

Reviewed By: Chillee

Pulled By: suo

fbshipit-source-id: d99d37699ec2e172e3798763e60aafe9a66ed6f4
2021-05-19 23:03:12 -07:00
Michael Suo
01d0eb9dac [package] Add an intern keyword (#57341)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/57341

Require that users be explicit about what they are going to be
interning. There are a lot of changes that are enabled by this. The new
overall scheme is:

PackageExporter maintains a dependency graph. Users can add to it,
either explicitly (by issuing a `save_*` call) or explicitly (through
dependency resolution). Users can also specify what action to take when
PackageExporter encounters a module (deny, intern, mock, extern).

Nothing (except pickles, tho that can be changed with a small amount
of work) is written to the zip archive until we are finalizing the
package. At that point, we consult the dependency graph and write out
the package exactly as it tells us to.

This accomplishes two things:
1. We can gather up *all* packaging errors instead of showing them one at a time.
2. We require that users be explicit about what's going in packages, which is a common request.

Differential Revision: D28114185

Test Plan: Imported from OSS

Reviewed By: SplitInfinity

Pulled By: suo

fbshipit-source-id: fa1abf1c26be42b14c7e7cf3403ecf336ad4fc12
2021-05-12 16:22:43 -07:00
Sam Estep
75024e228c Add lint for unqualified type: ignore (#56290)
Summary:
The other half of https://github.com/pytorch/pytorch/issues/56272.

Pull Request resolved: https://github.com/pytorch/pytorch/pull/56290

Test Plan:
CI should pass on the tip of this PR, and we know that the lint works because the following CI runs (before this PR was finished) failed:

- https://github.com/pytorch/pytorch/runs/2384511062
- https://github.com/pytorch/pytorch/actions/runs/765036024

Reviewed By: seemethere

Differential Revision: D27867219

Pulled By: samestep

fbshipit-source-id: e648f07b6822867e70833e23ddafe7fb7eaca235
2021-04-21 08:07:23 -07:00
Michael Suo
8d4e6c9570 [package] make GlobGroup a public concept (#56238)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/56238

It's already functionally public due to `extern` and `mock`, but
exposing the underlying implementation makes extending PackageExporter
easier.

Changed the underscores, expose on `torch.package`, add docs, etc.

Differential Revision: D27817013

Test Plan: Imported from OSS

Reviewed By: Lilyjjo

Pulled By: suo

fbshipit-source-id: e39199e7cb5242a8bfb815777e4bb82462864027
2021-04-16 13:31:48 -07:00
Michael Suo
8f68396462 [package] fix error handling with allow_empty (#56190)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/56190

Previously, if we had some code that did the following:
```
- pattern A, allow_empty=False
- save module B, but throws an exception for whatever reason
- save module that causes match against A
```

Then the resulting behavior would be:
1. exception thrown, which triggers `__close__` on `PackageExporter`
2. `PackageExporter` checks that all patterns are matched against, and sees that A was not matched.
3. Error is raised that we didn't match against pattern A.

This is confusing, since the *real* error that caused packaging to fail
occurred when trying to package module B, but it's being hidden by the
error about module A (even though if packaging module B had succeeded,
there would be no error).

Change it so that the behavior looks like:
1. exception thrown, which triggers `__close__` on `PackageExporter`
2. `PackageExporter` recognizes that an exception is happening and
immediately just returns control flow to the caller to handle the "real"
exception.

Differential Revision: D27803988

Test Plan: Imported from OSS

Reviewed By: guangyuwang

Pulled By: suo

fbshipit-source-id: f67b2e96165a0547c194a8bef1af1c185452173e
2021-04-15 20:16:43 -07:00
Meghan Lele
d58c00a5d8 [package] Make exporters write to buffer in fbcode (#54303)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/54303

**Summary**
Creating temporary files can cause problem in fbcode. This commit
updates the packaging tests so that exporters write to a memory
buffer when tests run in fbcode.

**Test Plan**
Continuous integration.

Test Plan: Imported from OSS

Reviewed By: suo

Differential Revision: D27180839

Pulled By: SplitInfinity

fbshipit-source-id: 75689d59448de2cd1595ef0ecec69e1bbcf9a96f
2021-03-19 19:59:35 -07:00
Michael Suo
741d0f41d6 [package] split tests (#53749)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/53749

Split up tests into cases that cover specific functionality. Goals:
1. Avoid the omnibus test file mess (see: test_jit.py) by imposing early
   structure and deliberately avoiding a generic TestPackage test case.
2. Encourage testing of individual APIs and components by example.
3. Hide the fake modules we created for these tests in their own folder.

You can either run the test files individually, or still use
test/test_package.py like before.

Also this isort + black formats all the tests.

Test Plan: Imported from OSS

Reviewed By: SplitInfinity

Differential Revision: D26958535

Pulled By: suo

fbshipit-source-id: 8a63048b95ca71f4f1aa94e53c48442686076034
2021-03-10 16:07:36 -08:00