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

Fix CI #592

Merged
merged 8 commits into from
Mar 1, 2024
Merged

Fix CI #592

merged 8 commits into from
Mar 1, 2024

Conversation

rth
Copy link
Collaborator

@rth rth commented Feb 29, 2024

  • Switch to fetching starting kit repos via HTTP rather than using git clone. As far as I can tell all those git clone in CI tests were triggering some github safety blocking the requests, leading the CI to hang.
  • Fix for leap year on 29 february,
    NOW.replace(year=NOW.year + 1),
E   ValueError: day is out of range for month 

Copy link

codecov bot commented Feb 29, 2024

Codecov Report

Attention: Patch coverage is 94.73684% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 93.48%. Comparing base (9b1d738) to head (63fd24f).
Report is 6 commits behind head on master.

Files Patch % Lines
ramp-database/ramp_database/testing.py 94.73% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #592      +/-   ##
==========================================
- Coverage   93.51%   93.48%   -0.04%     
==========================================
  Files         103      103              
  Lines        8976     8991      +15     
==========================================
+ Hits         8394     8405      +11     
- Misses        582      586       +4     

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

@rth rth changed the title WIP: Fix CI Fix CI Mar 1, 2024
@@ -172,8 +195,10 @@ def setup_ramp_kit_ramp_data(
'it, you need to set "force=True".'
)
shutil.rmtree(problem_data_path, ignore_errors=True)
ramp_data_url = "https://github.com/ramp-data/{}.git".format(problem_name)
Repo.clone_from(ramp_data_url, problem_data_path, **kwargs)
ramp_kit_url = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

ramp_data_url here instead of ramp_kit_url ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, good point thanks

Copy link
Collaborator

@frcaud frcaud left a comment

Choose a reason for hiding this comment

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

Thanks @rth ! LGTM
Just see the comment line 198 of testing.py

ramp_kit_url = (
f"https://github.com/ramp-data/{problem_name}/archive/refs/heads/master.zip"
)
_fetch_github_repo(ramp_kit_url, problem_data_path)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ramp_data_url also

@rth rth merged commit a924439 into paris-saclay-cds:master Mar 1, 2024
7 of 8 checks passed
@rth rth deleted the fix-CI branch March 1, 2024 10:18
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