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

Refactor git lookup #71

Closed
florian-jaeger opened this issue Apr 11, 2023 · 1 comment
Closed

Refactor git lookup #71

florian-jaeger opened this issue Apr 11, 2023 · 1 comment
Assignees
Labels
backlog Will be adressed in the future refactor Process should be refactored service Issue related to grader-service

Comments

@florian-jaeger
Copy link
Contributor

No description provided.

@florian-jaeger florian-jaeger added service Issue related to grader-service refactor Process should be refactored labels Apr 11, 2023
@adammccartney
Copy link
Member

So it felt like I made a little bit of progress with this over the past couple of days, today I pushed sha 5163fec with the most recent updates. Since making specific dataclasses for the various types of git repos, I had some further thoughts on how to deal with the very many calls to os.path.join(...) that are getting made all over the place still.

I was wondering what would be the implication of implementing __repr__ methods on each of our GitRepo classes that we now have. This would mean that we can simple call os.path(str(git_repo)) when we want to create the specific path. Instead of having to always call os.path.join(git_repo.root, git_repo.lecture_code, git_repo.assignment_id). I pushed a quick edit of these classes and they now look like this:

@dataclass
class GitRepoBasePath:
root: str
lecture_code: str
assignment_id: int
repo_type: str
def __repr__(self):
return f"""{self.root!r}/{self.lecture_code!r}/\
{self.assignment_id!r}/{self.repo_type!r}"""
class GitRepoSubmissionExtendedPath(GitRepoBasePath):
def __init__(self, root: str, lecture_code: str,
assignment_id: int, repo_type: str,
submission_id: int):
super().__init__(root, lecture_code, assignment_id, repo_type)
self.submission_id = submission_id
def __repr__(self):
return f"""{self.root!r}/{self.lecture_code!r}/{self.assignment_id!r}/\
{self.repo_type!r}/{self.submission_id!r}"""
class GitRepoUserExtendedPath(GitRepoBasePath):
def __init__(self, root: str, lecture_code: str,
assignment_id: int, repo_type: str,
username: str):
super().__init__(root, lecture_code, assignment_id, repo_type)
self.username = username
def __repr__(self):
return f"""{self.root!r}/{self.lecture_code!r}/{self.assignment_id!r}/\
{self.repo_type}/{self.username!r}"""

As I mentioned in the commit, I haven't tested them yet and also somehow I am not convinced that I've got the representation of the paths quite right - my inclusion of the self.root is possibly superfluous. I think the actually creation of the paths on the system is being done by GraderBaseHandler.construct_git_dir(). I'm not sure, but this seems to me that it might be happier living with the other git specific methods in GitBaseHandler. It also appears as if we are stripping and then re-appending the git pathlet in between receiving the request and setting up the path on our filesystem.

@adammccartney adammccartney added this to the Backlog milestone Sep 1, 2023
@adammccartney adammccartney added the backlog Will be adressed in the future label Sep 1, 2023
@meffmadd meffmadd removed this from the Backlog milestone Mar 13, 2024
@meffmadd meffmadd closed this as not planned Won't fix, can't repro, duplicate, stale Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Will be adressed in the future refactor Process should be refactored service Issue related to grader-service
Projects
None yet
Development

No branches or pull requests

3 participants