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

review / refactor tests/constants.py #2536

Open
ReimarBauer opened this issue Sep 19, 2024 · 4 comments
Open

review / refactor tests/constants.py #2536

ReimarBauer opened this issue Sep 19, 2024 · 4 comments
Labels
enhancement New feature or request tests
Milestone

Comments

@ReimarBauer
Copy link
Member

This looks to me currently a bit overengeenered. On github also each worker creates anyway a new tmp dir.
Is there any case on local development where we need a second information about the state of the git repository?

try:
    import git
except ImportError:
    SHA = ""
else:
    path = os.path.dirname(os.path.realpath(__file__))
    repo = git.Repo(path, search_parent_directories=True)
    logging.debug(path)
    try:
        # this is for local development important to get a fresh named tmpdir
        SHA = repo.head.object.hexsha[0:10]
    except (ValueError, BrokenPipeError) as ex:
        logging.debug("Unknown Problem: %s", ex)
        # mounted dir in docker container and owner is not root
        SHA = ""

ROOT_FS = TempFS(identifier=f"msui{SHA}")
@ReimarBauer ReimarBauer added enhancement New feature or request tests labels Sep 19, 2024
@MF-Vroom
Copy link

Catch specific exceptions instead of general ValueError and BrokenPipeError. Consider git.exc.InvalidGitRepositoryError

@YashaswiniTB
Copy link

Can I work on this issue?

@ReimarBauer ReimarBauer added this to the 10.0.0 milestone Sep 30, 2024
@ReimarBauer
Copy link
Member Author

Welcome @YashaswiniTB I assigned it to you.

@imakhan80
Copy link

This looks to me currently a bit overengeenered. On github also each worker creates anyway a new tmp dir. Is there any case on local development where we need a second information about the state of the git repository?

try:
    import git
except ImportError:
    SHA = ""
else:
    path = os.path.dirname(os.path.realpath(__file__))
    repo = git.Repo(path, search_parent_directories=True)
    logging.debug(path)
    try:
        # this is for local development important to get a fresh named tmpdir
        SHA = repo.head.object.hexsha[0:10]
    except (ValueError, BrokenPipeError) as ex:
        logging.debug("Unknown Problem: %s", ex)
        # mounted dir in docker container and owner is not root
        SHA = ""

ROOT_FS = TempFS(identifier=f"msui{SHA}")

The SHA is being used as an identifier for the TempFS, presumably to create unique and consistent temporary filesystems during local development.
However, as you pointed out, in many environments (like GitHub Actions), a new temporary directory is created for each run anyway, making this extra identifier potentially unnecessary.
For local development, having the SHA in the identifier could potentially help in tracking changes across different commits or branches, but this benefit might be minimal in most cases.
The current implementation has error handling for cases where the git repository information can't be retrieved, falling back to an empty string for the SHA.
import os
import tempfile
import logging

def get_git_sha():
try:
from git import Repo
except ImportError:
logging.debug("GitPython not installed, skipping SHA retrieval")
return ""

try:
    path = os.path.dirname(os.path.realpath(__file__))
    repo = Repo(path, search_parent_directories=True)
    sha = repo.head.object.hexsha[:10]
    logging.debug(f"Git SHA: {sha}")
    return sha
except Exception as ex:
    logging.debug(f"Failed to get Git SHA: {ex}")
    return ""

Use a unique identifier for the temporary filesystem

unique_id = f"msui{get_git_sha()}"
ROOT_FS = tempfile.mkdtemp(prefix=unique_id)

@YashaswiniTB YashaswiniTB removed their assignment Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tests
Projects
None yet
Development

No branches or pull requests

4 participants