Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added description of remaining endpoints of the gcbm api and resolved typos in the readme #129

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

AayushSaini101
Copy link
Contributor

Added description of remaining endpoints of the gcbm api and resolved typos in the readme.

AayushSaini101 and others added 7 commits June 4, 2022 16:13
Signed-off-by: AayushSaini101 <[email protected]>
Signed-off-by: AayushSaini101 <[email protected]>
Signed-off-by: AayushSaini101 <[email protected]>
Signed-off-by: AayushSaini101 <[email protected]>
Signed-off-by: AayushSaini101 <[email protected]>
Signed-off-by: AayushSaini101 <[email protected]>
@@ -342,6 +342,8 @@ def gcbm_download():
# Sanitize title
title = "".join(c for c in title if c.isalnum())
project_dir = f"{title}"
if not os.path.exists(f"{os.getcwd()}/input/{project_dir}"):
return {"error": "No such simulation exist for this {title}"}, 400
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the error status code, can be changed to 404 as 400 is used for a bad request and 404 is for a not found error. Let me know your thoughts on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think 404 is valid here because the simulation of the given title is not present. @padmajabhol What are your views on this issue.

Copy link
Member

@padmajabhol padmajabhol left a comment

Choose a reason for hiding this comment

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

You need to add CVE-2022-1966 and CVE-2022-29361 for the flint example build to app.

Signed-off-by: AayushSaini101 <[email protected]>
@AayushSaini101
Copy link
Contributor Author

@padmajabhol Done

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.

4 participants