Skip to content

Conversation

@hrideshmg
Copy link
Contributor

@hrideshmg hrideshmg commented Mar 18, 2025

Please prefix your pull request with one of the following: [FEATURE] [FIX] [IMPROVEMENT].

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

My familiarity with the project is as follows (check one):

  • I have never used the project.
  • I have used the project briefly.
  • I have used the project extensively, but have not contributed previously.
  • I am an active contributor to the project.

This PR addresses several bugs encountered during the initial setup of the platform on a fresh VPS. The deployment process was less than pleasant, and this fix aims to improve the experience by fixing a few of the errors and improving the documentation to add a few missing pieces (like the necessity of ccextractortester and webhooks).

Summary of code changes

  • The sample_db.py file was missing a few imports which were required for the creation of tables by SQLAlchemy. see docs.
  • The timestamp in one particular strptime() expects millisecond precision, however MySQL does not enable this by default. I also noticed that the millisecond format is not used anywhere else in the codebase apart from the initial setup run.
  • One division by zero and null value access error.

Comment on lines 1351 to 1352
start = datetime.datetime.strptime(parts[0], '%Y-%m-%d %H:%M:%S')
end = datetime.datetime.strptime(parts[-1], '%Y-%m-%d %H:%M:%S')
Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify why you changed these lines?

Copy link
Contributor Author

@hrideshmg hrideshmg Mar 18, 2025

Choose a reason for hiding this comment

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

This code was throwing an error citing a format mismatch when i ran this.

MySQL databases by default store the timestamps without millisecond precision (the .%f). The TestProgress model also doesn't have any configuration for said precision. I looked around and noticed that this was the only strptime() call in the codebase which expected this kind of precision.

Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit hesitant to merge this as is because I can't remember (and check right now) if we have the precision in the database or not.

Maybe wrap it in a try-except to catch it just to be sure? Try with precision first and then fallback if it fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that is probably the safest bet. This also broke tests apparently because SQLite does have precision and the test was reliant on the code being in the current state. I've pushed the fix 👍

@hrideshmg
Copy link
Contributor Author

I've fixed the isort error, the workflow checks should be fine now.

@hrideshmg hrideshmg force-pushed the deployment_fixes branch 3 times, most recently from d1292d9 to b122dee Compare March 18, 2025 20:51
@codecov
Copy link

codecov bot commented Mar 18, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 86.85%. Comparing base (19052be) to head (79ff180).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
mod_ci/controllers.py 42.85% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #926      +/-   ##
==========================================
- Coverage   86.91%   86.85%   -0.07%     
==========================================
  Files          35       35              
  Lines        3699     3704       +5     
  Branches      759      759              
==========================================
+ Hits         3215     3217       +2     
- Misses        348      351       +3     
  Partials      136      136              
Flag Coverage Δ
unittests 86.85% <50.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@canihavesomecoffee canihavesomecoffee merged commit 6e01bcd into CCExtractor:master Mar 20, 2025
5 of 7 checks passed
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.

2 participants