Guard against parser errors detected while migrating python code

[aparajit-pratap]

Purpose

https://jira.autodesk.com/browse/DYN-3102
Guard against crash due to parser errors detected while migrating python code. The following message is shown in the migration assistant in such a case:

Declarations

Check these if you believe they are true

• The codebase is in a better state after this PR
• Is documented according to the standards
• The level of testing this PR includes is appropriate - WIP
• User facing strings, if any, are extracted into *.resx files
• All tests pass using the self-service CI.
• Snapshot of UI changes, if any.
• Changes to the API follow Semantic Versioning and are documented in the API Changes document.
• This PR modifies some build requirements and the readme is updated

Reviewers

@Amoursol - any inputs on the error message (see screenshot).
@mmisol

[mmisol]

Just one comment.

On a general note, I don't think letting the user continue with the migration is a good idea in this case. We would create a backup with the invalid code, which is probably not useful.

[aparajit-pratap]

Just one comment.

On a general note, I don't think letting the user continue with the migration is a good idea in this case. We would create a backup with the invalid code, which is probably not useful.

Well, I guess the same could be said when there are no code changes needed between python2 and python3.

[mmisol]

@aparajit-pratap In my mind it's a different case.

When there are no changes for the current node, you can still say things are "in order" and other nodes can still present changes, so the backup is still useful. Also, remember you are changing the engine, so the backup should happen at this point.

However, if the syntax is incorrect then most likely things are not "in order". Maybe the user typed something by mistake before launching the migrator? Backing up the graph with that syntax error is what I believe is not useful. If the user needs to restore the backup, the graph will be invalid, which is probably not expected.

[aparajit-pratap]

@mmisol I wanted to show a message to the user saying there is an error in the code when he/she clicked the 2-3 migration icon. If we simply don't do anything then the user could be confused in the absence of any feedback. Perhaps I can continue to show the message in the migration assistant but disable the file backup?

[mmisol]

@aparajit-pratap I'm not saying we should not give feedback to the user, I'm just saying we shouldn't let them proceed. For this purpose we could, for instance, disable the accept button.

Beyond the backup, we should also avoid changing the engine. Otherwise, when we do backup we start from invalid content anyway. Preventing the migration seems the safest approach in this situation.

[aparajit-pratap]

@Amoursol do you have any feedback on the expected behaviour when there is a parser error in the code and the migration cannot proceed. Do we simply want to disable the migration assistant UI (not do anything), or show a message like the one above, or show some other message popup?

[Amoursol]

@aparajit-pratap - This is in the use case where users have non-compliant IronPython2 code they wish to migrate? i.e adding in a single line of blah blah blah not explicitly called out as a string causing the Python engine to fail?

If so, I agree that we should not allow them to run the 2->3 migration assistant (Disable 'accept' button) + provide a message stating why they cannot use the assistant (The message you have above is good if we change accept to cancel!)

[mmisol]

Some minor comments. The message one is just a suggestion, you might want to confirm with @Amoursol on the final wording.

[mmisol]

Technically, the engine will not be switched back. It should simply remain the same. Even though it doesn't make much sense, you could also launch the migrator with the CPython3 engine set.

How about something more on the lines of: The code migration cannot continue. Please click cancel and fix all errors before re-attempting it.

[Amoursol]

I'll file a new task - but we should disable the 2->3 button when you have the CPython3 engine set. It's confusing otherwise!

[aparajit-pratap]

If the engine is already CPython, I'm not doing anything special currently like switching it back to IronPython. So I'm removing the second sentence altogether and leaving it at that.