Fix Windows bug with save_restore_connector

[trias702]

Signed-off-by: Daniel Egert [email protected]

What does this PR do ?

Fixes a bug with saving Nemo models using model.save_to()

Collection: core

Changelog

• Changed some ordering in nemo/core/connectors/save_restore_connector.py

Usage

To trigger the bug, and check it's fixed:

from nemo.collections.asr.models import ASRModel

model = ASRModel.restore_from(...)

model.save_to(...)    # crashes on Windows without this fix

Before your PR is "Ready for review"

Pre checks:

• Make sure you read and followed Contributor guidelines
• Did you write any new necessary tests?
• Did you add or update any necessary documentation?
• Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
  • Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

• New Feature
• Bugfix
• Documentation

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

If you look at all of the uses of with tempfile.TemporaryDirectory() in save_restore_connector.py, you will see they all adhere to putting the tempfile context outer to the try-finally for the os.chdir call. However, this is not the case for _handle_artifacts which leads me to believe this is a bug where someone wrote _handle_artifacts without paying attention to the previous design pattern for tempfile context used throughout the same file. Unfortunately, deviating from that design pattern has catastrophic problems on Windows, but once we fix it to work like all the other entries in the same file, everything works correctly again. This is what I have done.

[titu1994]

Change seems fine. Just reordering of the try catch to always create the tempdir which makes sense. @ericharper for final review c

[ericharper]

LGTM. Thanks!