Skip to content

[Refactoring] - Eliminating Dead Code - #3316

Merged
VindeeR merged 1 commit into
masterfrom
removing_dead_code
Oct 1, 2026
Merged

VindeeR merged 1 commit into
masterfrom
removing_dead_code

Conversation

@VindeeR

@VindeeR VindeeR commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

@ntorqulu and I found some pieces of code that seems to be unused both at AS and API, since it has some tests it'll be removed too and hopefully it might help even if a bit with the time execution of the tests.

@dbeltrankyl I bieleve you might have done the refactor of this changes so could you confirm if it is okay to remove them?

Check List

  • I have read CONTRIBUTING.md.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to pyproject.toml.
  • Tests are included (or explain why tests are not needed).
  • Changelog entry included in CHANGELOG.md if this is a change that can affect users.
  • Documentation updated.
  • If this is a bug fix, PR should include a link to the issue (e.g. Closes #1234).

@VindeeR VindeeR added this to the 4.2.0 milestone Sep 30, 2026
@VindeeR
VindeeR requested a review from dbeltrankyl September 30, 2026 13:04
@VindeeR VindeeR self-assigned this Sep 30, 2026
@VindeeR VindeeR added enhancement New feature or request refactoring labels Sep 30, 2026
@dbeltrankyl

Copy link
Copy Markdown
Collaborator

Hello,

The code is there have it avaliable for both sqlalchemist and not sqlalchemist

#3308 already handles this I believe, as there we will only have Sqlalachemist

@VindeeR

VindeeR commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

I just checked the removed code at the branch you mentioned but I didn't see the implementation using the removed function from here.

Do you want to double check them and maybe even merge this to your branch if removing them makes sense. @dbeltrankyl

@dbeltrankyl

Copy link
Copy Markdown
Collaborator

Ah, I looked at the protocol stub only > get_last_job_data_dc_by_job_name_and_fail_counter this is what #3308 solves
and assumed that everything was related to that, sorry.

ExperimentHistory.get_finish_data_dc
ExperimentHistory.update_submit_time
Job._get_submit_data_dc_from_db
Job._get_finish_time_from_db

Is in both branches, right.

They can be delete I believe, these are a left-over of some refactor of the log_recovery ( there where too many iterations on that place) thanks

@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.14%. Comparing base (fb00cc5) to head (b35747f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3316      +/-   ##
==========================================
- Coverage   85.17%   85.14%   -0.04%     
==========================================
  Files         148      148              
  Lines       22132    22093      -39     
  Branches     3918     3918              
==========================================
- Hits        18850    18810      -40     
  Misses       2521     2521              
- Partials      761      762       +1     
Flag Coverage Δ
fast-tests 85.14% <ø> (-0.04%) ⬇️

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

Files with missing lines Coverage Δ
...database_managers/experiment_history_db_manager.py 89.31% <ø> (ø)
autosubmit/history/experiment_history.py 74.49% <ø> (-1.86%) ⬇️
autosubmit/job/job.py 89.12% <ø> (-0.04%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…since it has some tests it'll be removed too
@VindeeR
VindeeR force-pushed the removing_dead_code branch from 42fed09 to b35747f Compare October 1, 2026 07:16
@VindeeR
VindeeR requested a review from ntorqulu October 1, 2026 07:19

@kinow kinow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@VindeeR I think we talked about these functions in one of your other pull requests? Build is green, you said Núria checked this already, and Dani just confirmed these are OK to remove. +1 from me, approving! 💥

@VindeeR
VindeeR merged commit e461af5 into master Oct 1, 2026
20 of 21 checks passed
@VindeeR
VindeeR deleted the removing_dead_code branch October 1, 2026 07:53
@github-project-automation github-project-automation Bot moved this from Review to Done in Autosubmit project Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request refactoring

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants