Skip to content

#3 Use django app to query cve-search and display cve-ids - #4

Merged
pombredanne merged 10 commits into
developfrom
django_app
Jun 29, 2017
Merged

#3 Use django app to query cve-search and display cve-ids#4
pombredanne merged 10 commits into
developfrom
django_app

Conversation

@kartiksibal

Copy link
Copy Markdown
Contributor

Signed-off-by: Kartik Sibal kartiksibal@gmail.com

@kartiksibal kartiksibal changed the title #2 Use django app to query cve-search and display cve-ids #3 Use django app to query cve-search and display cve-ids Jun 18, 2017
Comment thread app/vulncode_app/views.py Outdated
data = api.output_cve_id(name=product)
return HttpResponse(data)

def product_ver(request, product, ver):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why using version in the api module and ver here?
Always prefer to use easy to read and unambiguous variable name.
Also be consistent with your naming within the codebase, if the content is the same, use the same name.
def product_ver(request, product, ver):
->
def product_version(request, product, version):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had primarily done it to sort of create a little differentiation, b/w what's being passed and what it is being checked to. But, your point is more valid. 😄

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sort of create a little differentiation should be avoided, consistency first! :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tdruez Gotcha! 😉

Comment thread app/vulncode_app/views.py Outdated

import vulncode_app.api_data as api

def product(request, product):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You are using package in the api_data module and product here, any particular reason?

@kartiksibal kartiksibal Jun 20, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Somewhat in contrast to cve-search. I'll switch 'em to package.

Comment thread app/vulncode_app/views.py
extracted_data = api.extract_fields(raw_data, fields_names, version=True)

return JsonResponse({'cve_id':api.ids,'summary':api.summary,'cvss':api.cvss})
return HttpResponse(json.dumps(extracted_data))

@kartiksibal kartiksibal Jun 21, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@pombredanne I had changed the data structure of the data, from lists to dictionary and this is sort of what I had implemented. The ordering works fine, but I think the HttpResponse ignores the indentation specified at the time of the JSON dump.
I had tried that before, but it didn't seem to work.

Do you think an alternate approach should be used here?

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do not commit .pyc and pycache_ fields
Add license headers.... and a few other points: use imperative style in your docstrings (Retrun rather than Returns)

Comment thread .cache/v/cache/lastfailed Outdated
@@ -0,0 +1,3 @@
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do not commit the .cache dir: it should be removed and the git ignored.

Comment thread app/app/settings.py
@@ -0,0 +1,120 @@
"""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing license header

kartiksibal and others added 7 commits June 27, 2017 23:07
Signed-off-by: Thomas Druez <tdruez@nexb.com>
Signed-off-by:Kartik Sibal <kartiksibal@gmail.com>
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
@pombredanne
pombredanne merged commit cbe9125 into develop Jun 29, 2017
@pombredanne
pombredanne deleted the django_app branch June 29, 2017 11:06
pombredanne pushed a commit that referenced this pull request Apr 2, 2025
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
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.

3 participants