-
-
Notifications
You must be signed in to change notification settings - Fork 649
uv in unit-tests.yml #3350 #3401
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
base: master
Are you sure you want to change the base?
Conversation
.github/workflows/unit-tests.yml
Outdated
CLEARML_NO_DEFAULT_SERVER: 1 | ||
CLEARML_API_HOST: "http://localhost" | ||
CLEARML_WEB_HOST: "http://localhost" | ||
CLEARML_FILES_HOST: "http://localhost" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this vs CLEARML_OFFLINE_MODE=1
, https://clear.ml/docs/latest/docs/guides/set_offline/#setting-task-to-offline-mode
ignite/ignite/handlers/clearml_logger.py
Lines 152 to 167 in 5caecf2
def set_bypass_mode(cls, bypass: bool) -> None: | |
""" | |
Set ``clearml.Task`` to offline mode. | |
Will bypass all outside communication, and will save all data and logs to a local session folder. | |
Should only be used in "standalone mode", when there is no access to the *clearml-server*. | |
Args: | |
bypass: If ``True``, all outside communication is skipped. | |
Data and logs will be stored in a local session folder. | |
For more information, please refer to `ClearML docs | |
<https://clear.ml/docs/latest/docs/clearml_sdk/task_sdk/#offline-mode>`_. | |
""" | |
from clearml import Task | |
setattr(cls, "_bypass", bypass) | |
Task.set_offline(offline_mode=bypass) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand that, but without
CLEARML_API_HOST: "http://localhost"
CLEARML_WEB_HOST: "http://localhost"
CLEARML_FILES_HOST: "http://localhost"
the following error is produced:
FAILED tests/ignite/contrib/engines/test_common.py::test_setup_clearml_logging - ValueError: ClearML configuration could not be found (missing `~/clearml.conf` or Environment CLEARML_API_HOST)
.github/workflows/unit-tests.yml
Outdated
|
||
- name: Check code formatting | ||
run: | | ||
bash ./tests/run_code_style.sh install | ||
uv pip install --upgrade flake8 "black==24.10.0" "usort==1.0.8.post1" "ufmt==2.7.3" "mypy" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is bad, once we need to upgrade the version of one of these tools we need to do that in +1 place now.
In future I wanted to remove ./tests/run_code_style.sh
and use pre-commit
such that we can have only one file showing the used tools and the versions. But this is for another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that this is not ideal. If you have ideas for an alternative solution, I would gladly explore it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vfdev-5 redone with pre-commit. seems to be working fine
No description provided.