Skip to content

issue #31#32

Closed
parteekcoder wants to merge 5 commits intoControlCore-Project:devfrom
parteekcoder:dev
Closed

issue #31#32
parteekcoder wants to merge 5 commits intoControlCore-Project:devfrom
parteekcoder:dev

Conversation

@parteekcoder
Copy link
Contributor

@parteekcoder parteekcoder commented Feb 18, 2023

issue #31

issue #31 solved ,

We have to enable shell=True

Also There is One more bug that is if os.path.exists(dir_path) it will not enter if block and resp will not be defined and it also produces
@parteekcoder
Copy link
Contributor Author

issue #31

Currently I am testing some more and will update this PR more

when it is completed I will tell you

To run the FRI as a server:
````
$ cd conore
$ cd conore/fri
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was an issue in the dev branch. It was updated in the main branch. Now I have merged the pull request. So this README should be fixed now.

resp = jsonify({'message': 'There is an Error'})
resp.status_code = 500
call(["./build"], cwd=dir_path)
else:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the PRs to be minimal. If you are addressing a bug, the PR should just address that, instead of making the logs more verbose, like here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I put an else block here as if
If block is not executed then resp object will not be defined and will give error , as resp object is only defined inside IF block

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aha, sorry, my bad. You are right.

dir_path = os.path.abspath(os.path.join(concore_path, graphml_file)) #path for ./build
if not os.path.exists(dir_path):
proc = call(["./makestudy", makestudy_dir], cwd=concore_path)
proc = call(["makestudy", makestudy_dir],shell=True, cwd=concore_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. So this fix should be repeated for all the methods (clear, run, ...) I believe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I will update soon

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@parteekcoder
Copy link
Contributor Author

@pradeeban You can review know and if there is anything please let me know

@parteekcoder
Copy link
Contributor Author

I will also test it on more OS and will tell you if anything breaking

@pradeeban
Copy link
Member

Duplicate of #37. Closing this one in favor of the latest PR.

@pradeeban pradeeban closed this Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants