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 scheduled inputs #634

Merged
merged 4 commits into from
Jan 15, 2025
Merged

Fix scheduled inputs #634

merged 4 commits into from
Jan 15, 2025

Conversation

averissimo
Copy link
Contributor

@averissimo averissimo commented Jan 9, 2025

Part of insightsengineering/nestdevs-tasks#98

extra-deps was badly formed

Signed-off-by: André Veríssimo <[email protected]>
@averissimo averissimo added the core label Jan 9, 2025
@averissimo
Copy link
Contributor Author

averissimo commented Jan 9, 2025

There's still an error with min-cohort strategy, but it seems like a caching issue. It might go away without any action.

I've already tried removing the action's cache and there was no change. I'm testing it locally.

@averissimo averissimo marked this pull request as draft January 9, 2025 17:46
Copy link
Contributor

github-actions bot commented Jan 9, 2025

Unit Tests Summary

  1 files   29 suites   26s ⏱️
367 tests 367 ✅ 0 💤 0 ❌
819 runs  819 ✅ 0 💤 0 ❌

Results for commit ef9cbf3.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jan 9, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
FilteredData 💔 $9.75$ $+1.85$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
FilteredData 💔 $0.90$ $+1.22$ srv_active_clicking_remove_all_button_clears_filters

Results for commit ab22008

♻️ This comment has been updated with latest results.

@m7pr
Copy link
Contributor

m7pr commented Jan 13, 2025

@m7pr
Copy link
Contributor

m7pr commented Jan 14, 2025

min_cohort still filing due to

Error : ! Could not solve package dependencies:
* deps::/tmp/RtmpBgzJAD/filec0a3043ed:
  * Can't install dependency SummarizedExperiment
  * Can't install dependency bioc::SummarizedExperiment
* SummarizedExperiment: Can't install dependency MatrixGenerics (>= 1.1.3)
* MatrixGenerics: Can't install dependency matrixStats (>= 1.4.1)
* bioc::SummarizedExperiment: Can't install dependency MatrixGenerics (>= 1.1.3)
── Create lockfile... ──────────────────────────────────────────────────────────
Error : ! Could not solve package dependencies:
* deps::/tmp/RtmpBgzJAD/filec0a3043ed:
  * Can't install dependency SummarizedExperiment
  * Can't install dependency bioc::SummarizedExperiment
* SummarizedExperiment: Can't install dependency MatrixGenerics (>= 1.1.3)
* MatrixGenerics: Can't install dependency matrixStats (>= 1.4.1)
* bioc::SummarizedExperiment: Can't install dependency MatrixGenerics (>= 1.1.3)

@m7pr m7pr self-assigned this Jan 14, 2025
@m7pr
Copy link
Contributor

m7pr commented Jan 15, 2025

I needed to specify versions of bioc packages as the job actually installed very old packages. Now with the versions specified, the new job is satisfied for all strategies besides release (teal.logger is not yet released).

https://github.com/insightsengineering/teal.slice/actions/runs/12784156379/job/35636608638

I will try to remove bioc::MatrixGenerics from Config/Needs/verdepcheck from DESCRIPTION file, as this was added through this PR but might not be 100% needed. However, this is ready for the review

@m7pr m7pr marked this pull request as ready for review January 15, 2025 08:30
@m7pr m7pr requested a review from llrs-roche January 15, 2025 08:30
Copy link
Contributor

@llrs-roche llrs-roche left a comment

Choose a reason for hiding this comment

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

The matrix changes make sense and the versions requested are current released versions of packages. Not sure why bioc::MatrixGenerics was initially needed, but I think it is better to leave it out

@m7pr m7pr merged commit b52deaa into main Jan 15, 2025
30 checks passed
@m7pr m7pr deleted the scheduled_fix branch January 15, 2025 09:05
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants