Removing BuildAction table and related elements. #709
Conversation
|
I was wondering why are we removing the BuildAction? But the BuildAction (which should be named AnalysisAction in the first place) contains other useful information like how long did a certain action take? What was the command? We can use this information for diagnosing performance problems or rerunning actions in debug mode for additional diagnostics. So my question is, do we loose these functionalities with this change, or are they going to work after this change just using information from other sources? |
|
With the current database change/cleanup we will loose some of the debug functionality which was based on the data in the database. There is already some improvement in the error handling during analysis #679 |
| self.buildaction.analyzer_type, | ||
| source_file_name) | ||
|
|
||
| assert self.analyzer_returncode == 0 |
There was a problem hiding this comment.
Are you sure that this piece of error checking is related to build action handling?
| @@ -45,104 +45,6 @@ class CheckerReportHandler(object): | |||
| information to the database. | |||
| """ | |||
|
|
|||
| def __sequence_deleter(self, table, first_id): | |||
There was a problem hiding this comment.
Are you sure you do not need this? To delete the path of bug reports.
There was a problem hiding this comment.
The bug path representation also changes: instead of linked list we are ordering by an enumeration column, so no sequence deleter is needed.
5e89230 to
288e129
Compare
288e129 to
edeb4b6
Compare
api/report_storage_server.thrift
Outdated
| @@ -52,16 +52,8 @@ service CheckerReport { | |||
|
|
|||
| // The next few following functions must be called via the same connection. | |||
There was a problem hiding this comment.
Is this still valid?
I mean do we really want to handle storage transactions (storage of reports belonging to the same run instance) based on connections? What if a connection is breaks down due to network error? Will the run under being updated be broken too?
Wouldn't it be more safe to handle transactions independently of connections?
Something like:
i64 openStorageTransaction(run_id) //returns a run storage transaction id
addReport(...)
addReport(...)
closeStorageTransaction(i64 storage_transaction_id)
when the closeTransaction is called, we set the status of the reports that were not present in the session would be set to resolved.
There was a problem hiding this comment.
This will be the issue of #724.
In this commit the BuildAction is removed as a concept. This also means that update mode doesn't work any more, because a report is connected to a run instead of the build action. If a report is already found in the run then it won't be updated, but skipped. Later in this case the detection status of the report will be updated.
edeb4b6 to
c0bdb0a
Compare
In this commit the BuildAction is removed as a concept. This also means
that update mode doesn't work any more, because a report is connected to
a run instead of the build action. If a report is already found in the
run then it won't be updated, but skipped. Later in this case the
detection status of the report will be updated.