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

[server] Store, show and filter by analyzer name #2836

Merged

Conversation

csordasmarton
Copy link
Contributor

@csordasmarton csordasmarton commented Jul 14, 2020

resolves #2717

@csordasmarton csordasmarton added enhancement 🌟 API change 📄 Content of patch changes API! database 🗄️ Issues related to the database schema. test ☑️ Adding or refactoring tests documentation 📖 Changes to documentation. server 🖥️ labels Jul 14, 2020
@csordasmarton csordasmarton added this to the release 6.14.0 milestone Jul 14, 2020
@csordasmarton csordasmarton requested review from gyorb and bruntib July 14, 2020 12:13
@csordasmarton csordasmarton requested a review from dkrupp as a code owner July 14, 2020 12:13
@csordasmarton csordasmarton force-pushed the show_report_analyzer_name branch 2 times, most recently from e063cca to e921a26 Compare August 25, 2020 07:44
Copy link
Contributor

@bruntib bruntib left a comment

Choose a reason for hiding this comment

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

If I try rebuilding the package and starting the webserver with an existing database, I get an exception from alembic and the server doesn't start.

Comment on lines 637 to 640
Filter results by analyzer names. The analyzer name
can contain multiple * quantifiers which matches any
number of characters (zero or more). So for example
"clang*" will matches "clangsa" and "clang-tidy".
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Filter results by analyzer names. The analyzer name
can contain multiple * quantifiers which matches any
number of characters (zero or more). So for example
"clang*" will matches "clangsa" and "clang-tidy".
Filter results by analyzer names. The analyzer name
can contain multiple * quantifiers which match any
number of characters (zero or more). So for example
"clang*" will match "clangsa" and "clang-tidy".

@@ -25,7 +25,7 @@ class Report(object):
from the path section for easier skip/suppression handling
and result processing.
"""
def __init__(self, main, bugpath, files):
def __init__(self, main, bugpath, files, metadata):
Copy link
Contributor

Choose a reason for hiding this comment

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

metadata could have None as default value. It is called with None several times.

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 set this parameter default value to None.

By the way we call this with None in two places but in the following way:

metadata = None
Report(..., metadata)

So from this I can see explicitly that no metadata is given to the Report class.

Copy link
Contributor

@bruntib bruntib Aug 31, 2020

Choose a reason for hiding this comment

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

I see. Does this two-line version have documentation reasons instead of Report(..., None)? You may consider using Report(..., metadata=None) if you want to see parameter name at function call.

@csordasmarton csordasmarton force-pushed the show_report_analyzer_name branch from e921a26 to 05760b9 Compare August 27, 2020 07:20
@csordasmarton csordasmarton requested a review from bruntib August 27, 2020 07:22
@csordasmarton csordasmarton force-pushed the show_report_analyzer_name branch 4 times, most recently from ff4bcab to 497e3e2 Compare August 31, 2020 11:04

limit = verify_limit_range(limit)

results = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this contain analyzers as keys and 0 as value? Does it work if empty dict is returned?

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, it will work, for example we do the same thing in getSeverityCounts API function.

For example if the database is empty, so there isn't any report in the database then this function will return an empty dictionary.

This function returns a dictionary where the keys are analyzer names which can be found in the database.

@csordasmarton csordasmarton force-pushed the show_report_analyzer_name branch from 497e3e2 to 3c733ee Compare September 1, 2020 10:42
@csordasmarton csordasmarton force-pushed the show_report_analyzer_name branch 6 times, most recently from 942f6b5 to fda1fa4 Compare September 1, 2020 13:23
@csordasmarton csordasmarton force-pushed the show_report_analyzer_name branch from fda1fa4 to fe4d9f3 Compare September 1, 2020 13:36
@gyorb
Copy link
Contributor

gyorb commented Sep 1, 2020

Codacy Here is an overview of what got changed by this pull request:

Issues
======
- Added 1
           

Complexity increasing per file
==============================
- web/server/codechecker_server/migrations/report/versions/af5d8a21c1e4_add_analyzer_name_for_report.py  1
- web/client/codechecker_client/cmd_line_client.py  1
- web/server/codechecker_server/api/report_server.py  2
         

Clones added
============
- web/server/codechecker_server/api/report_server.py  6
         

See the complete overview on Codacy

@csordasmarton csordasmarton merged commit ffb9a31 into Ericsson:master Sep 2, 2020
@csordasmarton csordasmarton deleted the show_report_analyzer_name branch November 6, 2020 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change 📄 Content of patch changes API! database 🗄️ Issues related to the database schema. documentation 📖 Changes to documentation. enhancement 🌟 server 🖥️ test ☑️ Adding or refactoring tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show analyzer name alongside the reports
3 participants