search

FREVA-CLINT / freva

The Free Evaluation System Framework (FreVa)
Other
10 stars 3 forks source link

Initial pull request focused on integrating pre-commit and addressing several minor issues. #190

Closed MoSHad91 closed 2 months ago

MoSHad91 commented 3 months ago

This PR introduces pre-commit integration and fixes a number of minor issues. Detailed updates are documented in the CHANGELOG.md.

MoSHad91 commented 3 months ago

Dear @antarcticrainforest, I've completed my current work on this repository, addressing the issues you previously mentioned in the pull request. Additionally, I've introduced some new tests to enhance our coverage. Looking ahead, I plan to achieve full coverage after completing my learning and review of the freva-deployment. by the way I added an if condition for docs to not be running on none main branches, because of this, the doc service is blocked. I would greatly appreciate your review of the changes. If everything is in order, could you please merge the request?

MoSHad91 commented 2 months ago

Dear @antarcticrainforest,

I've found the issue affecting failure (freezed) of docs job in pipeline, which mirrors the same problem we encountered in a unit test where no status other than scheduled could be get when batchmode=True was enabled.

Identified Issues:

  1. The wait function in utils.py lacks a timeout parameter, causing it to hang indefinitely without raising an error. here is the line that keep us waited forever ...

  2. Another related issue involves a 60-second wait which, upon passing the conditional in the wait function, leads to a Plugin did not finish error as detailed here.

Solution:

A straightforward fix to get the documentation building process working is to simply remove these two problematic wait lines. This change has proven to successfully generate the docs in my tests.

However, for a more fundamental solution, we need to investigate why the Plugin status remains stuck at scheduled when batchmode=True. This might involve reviewing how batchmode is handled or making deeper changes to our plugin management logic. For instance:

res = freva.run_plugin("dummypluginfolders", batchmode=True)

If you agree I will comment wait in these two mentioned docstrings to pipeline works for now to be able to merge, then to investigate deeper, we can make an issue to tackle the problem.

antarcticrainforest commented 2 months ago

That is interesting. I do have a suspicion. I'll check tomorrow morning the docs config. Specifically what sheduler is used when the batchmode=True option gets involved.

MoSHad91 commented 2 months ago

thanks @antarcticrainforest after seven months finally we got the green pipeline in all services :). I think one of your review requests changrs is just stuck there and has blocked the Merging https://github.com/FREVA-CLINT/freva/pull/190#pullrequestreview-1989884357

MoSHad91 commented 2 months ago

@antarcticrainforest btw one docs action to main branch is still running: https://github.com/FREVA-CLINT/freva/actions/runs/9061866502/job/24894525377

antarcticrainforest commented 2 months ago

@antarcticrainforest btw one docs action to main branch is still running: https://github.com/FREVA-CLINT/freva/actions/runs/9061866502/job/24894525377

Cancelled.