4651 : adding a change to fix database is locked error #4960
Conversation
RADXIshan
force-pushed
the
RADXIshan/4651
branch
from
ba84d6b to
a5bcd43
Compare
cve_bin_tool/cvedb.py
Outdated
| LOGGER.error("Database cursor does not exist") | ||
| raise CVEDBError | ||
| return cursor | ||
| def db_open_and_get_cursor(self): |
There was a problem hiding this comment.
We should keep the type hint here.
cve_bin_tool/cvedb.py
Outdated
| retries = 0 | ||
| while retries < MAX_RETRIES: | ||
| try: | ||
| self.connection = sqlite3.connect(self.dbpath, timeout=10) # Extend timeout |
There was a problem hiding this comment.
I'm not confident we should extend the timeout in addition to adding the retry logic, but I could be convinced. What's the thinking behind this change?
cve_bin_tool/cvedb.py
Outdated
| if "database is locked" in str(e): | ||
| retries += 1 | ||
| wait_time = RETRY_DELAY * (2 ** retries) # Exponential backoff | ||
| print(f"Database locked, retrying in {wait_time:.2f} seconds...") |
There was a problem hiding this comment.
Please make use of the LOGGER object here instead of a bare print call.
cve_bin_tool/cvedb.py
Outdated
| print(f"Database locked, retrying in {wait_time:.2f} seconds...") | ||
| time.sleep(wait_time) | ||
| else: | ||
| raise # Raise other errors |
There was a problem hiding this comment.
I think for this raise and the one right below it, we should use LOGGER.error and raise CVEDBError instead of raising whatever sqlite3 gives us. That will keep the existing error handler logic usable if it's ever needed for something.
cve_bin_tool/cvedb.py
Outdated
| while retries < MAX_RETRIES: | ||
| try: | ||
| self.connection = sqlite3.connect(self.dbpath, timeout=10) # Extend timeout | ||
| self.connection.execute("PRAGMA journal_mode=WAL;") # Improve concurrency |
There was a problem hiding this comment.
Trying to think of side effects this might cause. This will create some extra files on disk next to our database, right? I don't think there would be issues with this change, but I defer to the other maintainers who might be more familiar with the SQLite stuff...
|
Thanks for working on this, @RADXIshan! I think this could help with the intermittent database issues. Please look over my review comments when you get a chance. |
|
Thanks for the review, I will look into the issues you have mentioned and
try to fix them . I would be happy to collaborate with you on this project
and i really appreciate some guidance as I have just started getting into
open source.
…On Fri, 28 Mar 2025 at 11:46 PM, stvml ***@***.***> wrote:
Thanks for working on this, @RADXIshan <https://github.com/RADXIshan>! I
think this could help with the intermittent database issues. Please look
over my review comments when you get a chance.
—
Reply to this email directly, view it on GitHub
<#4960 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BNLSALJZT5LXBCDH5OMYBG32WWGXTAVCNFSM6AAAAABZS6KZ6GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRSGEYDIMJRHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: stvml]*stvml* left a comment (intel/cve-bin-tool#4960)
<#4960 (comment)>
Thanks for working on this, @RADXIshan <https://github.com/RADXIshan>! I
think this could help with the intermittent database issues. Please look
over my review comments when you get a chance.
—
Reply to this email directly, view it on GitHub
<#4960 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BNLSALJZT5LXBCDH5OMYBG32WWGXTAVCNFSM6AAAAABZS6KZ6GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRSGEYDIMJRHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I have removed '.DS_Store' from my PR, let me know if any other changes are needed |
cve_bin_tool/cvedb.py
Outdated
| while retries < MAX_RETRIES: | ||
| try: | ||
| self.connection = sqlite3.connect(self.dbpath, timeout=10) # Extend timeout for better handling | ||
| return self.connection.cursor() |
There was a problem hiding this comment.
Sorry, one last change here... I think we should keep the existing check for if cursor is None before we return the result of this call. Not sure if this error ever happens in the real world, but it's better to preserve logic that we aren't sure about :)
Fixed the the required changes that were requested
One tab changed the whole indentation of the code and i didnt even realise :), thankss for the check i think now it will work 👍 |
|
Please have a look at this section of CONTRIBUTING.md for information about the linters this project uses. Highly recommend using |
ok i will go through it , this all is kind of new to me thats why i got confused , thanks for the guidance! |
I have tried implementing as much as i understood and i think the errors from my end are fixed now , i went through the pre commit command and fixed the errors, it is showing some errors in some files which i have not touched ,like its showing that those files have not gone through much tests, but i fixed everything for my own file , can please check once and thanks a lot for guiding me through everything i really love working with you, lemme know if i am going wrong somewhere and how i can fix that :) |
i am not able to fix this can you please help me with it
|
Made changes in the file cvedb.py to fix the issue 4651