Skip to content
This repository has been archived by the owner on Feb 9, 2021. It is now read-only.

First steps towards fixing CQGI Python 3 compatibility. #224

Merged
merged 7 commits into from
Feb 9, 2018
Merged

Conversation

jmwright
Copy link
Collaborator

@jmwright jmwright commented Feb 8, 2018

WIP: Please do not merge yet. I'm putting this out there to communicate what I'm doing, run the full sets of tests against my code, and allow @dcowden and @adam-urbanczyk to make comments as these fixes progress. These errors are down in the AST parts of the code, and there seem to be some subtle things going on, so it's taking me awhile to straighten things out.

@coveralls
Copy link

coveralls commented Feb 8, 2018

Coverage Status

Coverage decreased (-0.3%) to 86.885% when pulling fbac7f1 on cqgi_tests into ba6d211 on master.

@jmwright
Copy link
Collaborator Author

jmwright commented Feb 8, 2018

@adam-urbanczyk @dcowden This is ready to be checked over. If you approve of the way I fixed the failures, we should be able to merge this to get our builds passing again.

@adam-urbanczyk
Copy link
Contributor

@jmwright to me it looks OK, but I have to admit I do not understand cqgi implementation fully.

@dcowden
Copy link
Owner

dcowden commented Feb 9, 2018

@jmwright this looks ok as long as the tests pass..

@adam-urbanczyk cqgi the goal of cqgi is to provide a common way for tools to execute cqgi scripts, in a way that scripts don't need to be modified to suit the tool, and so that its easy for scripts to 'just work' without a lot of work by the script developer

In very short, CQGI does these steps:

  • parse the script, check it for syntax errors
  • make an AST tree of the script, and using that tree:
    • find variables that are set to contants, and tabulate their names and types
  • return a structure to the calling environment, allowing it to set variables to new values
  • when the calling environment runs the script, we run the script in a secure sandbox that doesnt allow nasty other builtins:
    • inject the show_object() method so that it can be called by the script
    • replace the vars with the new values, and capture the output objects
    • run the script
    • capture the output and give it back to the calling environment

You can read more here

@jmwright jmwright merged commit 465f291 into master Feb 9, 2018
@adam-urbanczyk
Copy link
Contributor

@dcowden thanks!

BTW I think #203 can be closed.

@jmwright jmwright mentioned this pull request Feb 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants