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

Facade repo path changes #2186

Merged
merged 11 commits into from Feb 23, 2023
Merged

Facade repo path changes #2186

merged 11 commits into from Feb 23, 2023

Conversation

IsaacMilarky
Copy link
Contributor

@IsaacMilarky IsaacMilarky commented Feb 22, 2023

Description

  • Facade no longer detects collision based on the repo table's repo_path and repo_name fields and instead looks inside the facade directory to detect collision. If collision is detected the repo is set to 'Update' and reset
  • Dependency model no longer relies on repo_path and repo_name fields and instead uses repo_git
  • Fix detection of repositories being moved to work with the new collection hooks. The prelim phase is still enabled/disabled through the same means but now has specific celery tasks defined for each collection hook. This is necessary because this task has to manage a special condition of collection and needs access to affect the CollectionStatus on the proper fields relating to the corresponding collection hook.

Signed commits

  • Yes, I signed my commits.

…l replaces these fields with slugs (ex: augur becomes augur-1) if they already are cloned and they are attemped to be initialized again. This shouldn't happen however

Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
…m it if it exists

Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
…m it if it exists

Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Copy link
Contributor

@ABrain7710 ABrain7710 left a comment

Choose a reason for hiding this comment

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

If I am understanding these changes correctly, it looks like Facade is still setting the repo_name and repo_path, it just doesn't depend on those fields to be in a certain state anymore. I think that we should be filling the repo_path and repo_name whenever a repo is added, so that we can be certain it existing since any repo that is added will have them set. The method to put this in is in called insert and is on the Repo class in the augur_data.py file.

@IsaacMilarky
Copy link
Contributor Author

If I am understanding these changes correctly, it looks like Facade is still setting the repo_name and repo_path, it just doesn't depend on those fields to be in a certain state anymore. I think that we should be filling the repo_path and repo_name whenever a repo is added, so that we can be certain it existing since any repo that is added will have them set. The method to put this in is in called insert and is on the Repo class in the augur_data.py file.

Sounds good. I didn't know it was that simple with our orm setup.

IsaacMilarky and others added 2 commits February 22, 2023 15:55
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Andrew Brain <61482022+ABrain7710@users.noreply.github.com>
Copy link
Contributor

@ABrain7710 ABrain7710 left a comment

Choose a reason for hiding this comment

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

Do we still need the UPDATE query on line 96 of facade05repofetch.py that sets the repo_name and repo_path? Since we are inserting the repo_name and repo_path with every new repo. Also I think there should be a revision script that inserts the repo_name and repo_path for any repos that don't have the repo_path or repo_name

@IsaacMilarky
Copy link
Contributor Author

Do we still need the UPDATE query on line 96 of facade05repofetch.py that sets the repo_name and repo_path? Since we are inserting the repo_name and repo_path with every new repo. Also I think there should be a revision script that inserts the repo_name and repo_path for any repos that don't have the repo_path or repo_name

I was reluctant to remove those updates since if we remove them we are trusting our regex to always match the way that we parse the repo name and path for every possible git url to the way that facade gets it from the git log. It should work without the update statements as long as we are mindful of these fields corresponding to facade's repo directory. I agree about the revision script I can add that.

Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@ABrain7710
Copy link
Contributor

ABrain7710 commented Feb 23, 2023

@IsaacMilarky Okay, so does this mean Facade still relies on the repo_name and repo_path it just doesn't depend on them being NULL? Also it seems if we are worried about the regex not producing the same results as Facade then we should put the way Facade is parsing it into a function, and then use that function in the Repo.insert() method and in the schema script. Then we would be confident that it is parsing them the way Facade needs them.

@IsaacMilarky
Copy link
Contributor Author

@IsaacMilarky Okay, so does this mean Facade still relies on the repo_name and repo_path it just doesn't depend on them being NULL? Also it seems if we are worried about the regex not producing the same results as Facade then we should put the way Facade is parsing it into a function, and then use that function in the Repo.insert() function and in the schema script. Then we would be confident that it is parsing them the way Facade needs them.

Facade still needs to keep track of the file that it has cloned a repo from previously for how we are doing collection. We can't really do it the same way that facade does it when they are inserted since that relies on the git repo being cloned onto disk. It does python indexing and some regex to get the path and name once the repo has been cloned.

Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky IsaacMilarky merged commit 6dfd48b into dev Feb 23, 2023
@IsaacMilarky IsaacMilarky deleted the facade-repo-path-changes branch February 23, 2023 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants