/snapcraft

pluginhandler: don't clobber path on local import failure #1361

Merged
merged 3 commits into from Jun 28, 2017

Conversation

Reviewers

@elopio elopio

@kyrofa kyrofa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  2.33
4 participants
@zyga @elopio @kyrofa @sergiusens
@zyga
Contributor

zyga commented Jun 9, 2017

The helper for importing local plugins would clobber sys.path if the
import failed. This is fixed with a simple try/finally statement.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com
@kyrofa

kyrofa approved these changes Jun 9, 2017
View changes

Good catch, thanks @zyga. 👍 from me, although this might be worth a test. @elopio, thoughts?

zyga and others added some commits Jun 9, 2017

@zyga @elopio
pluginhandler: don't clobber path on local import failure 
The helper for importing local plugins would clobber sys.path if the
import failed. This is fixed with a simple try/finally statement.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
7280253
@elopio
add an assertion
5f77757
@elopio
Member

elopio commented Jun 23, 2017

I added an assertion that fails without @zyga's fix. With this, I'm +1.

@elopio elopio merged commit c7447f8 into snapcore:master Jun 28, 2017
1 check passed

1 check passed

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

@sergiusens sergiusens added this to the 2.33 milestone Jun 28, 2017

kalikiana added a commit to kalikiana/snapcraft that referenced this pull request Aug 3, 2017

@zyga @kalikiana
pluginhandler: don't clobber path on local import failure (#1361) 
The helper for importing local plugins would clobber sys.path if the
import failed. This is fixed with a simple try/finally statement.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
f864d1b

kalikiana added a commit to kalikiana/snapcraft that referenced this pull request Sep 21, 2017

@zyga @kalikiana
pluginhandler: don't clobber path on local import failure (#1361) 
The helper for importing local plugins would clobber sys.path if the
import failed. This is fixed with a simple try/finally statement.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
8925470
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment