Support converting repository type #6469
Conversation
github-actions
bot
added
the
group/backend
LucasG0
force-pushed
the
lgu-convert-repos
branch
from
9b02ae9 to
389f21b
Compare
CodSpeed Performance ReportMerging #6469 will not alter performanceComparing Summary
|
LucasG0
force-pushed
the
lgu-convert-repos
branch
4 times, most recently
from
48540b8 to
1cb2a46
Compare
| parameters={"model": git_repo_add_model}, | ||
| ) | ||
|
|
||
| # TODO Validate that the creation of the repository went as expected |
There was a problem hiding this comment.
I think you can just delete this
There was a problem hiding this comment.
I wonder if we should be adding more files under infrahub.core, at least we should have a discussion around it. I think we can keep these changes here until we have decided but it but we should have some consistent thought around where things are placed.
backend/tests/functional/convert_object_type/test_convert_repositories.py
Outdated
Show resolved
Hide resolved
backend/tests/functional/convert_object_type/test_convert_repositories.py
Outdated
Show resolved
Hide resolved
LucasG0
force-pushed
the
lgu-convert-repos
branch
from
356d6d3 to
4bdb07f
Compare
There was a problem hiding this comment.
I think this looks good. I have two comments around the changes.
-
Do we need to lock the repository for git operations while we are doing this conversion? Potentially there will be some tasks that could fail during the conversion. I.e. if the task starts off and expects a managed repository to exist but then it ends up being a read-only repository before the task has completed. The a worse problem would be if we're syncing the repo and a new commit is observed. It might be that this operation would simply fail, alternatively we'd wait until a mutation lock has been released. Do you have any thoughts around that?
-
For a repository that lives on multiple branches where the commit value is different on a number of the branches, given the mappings we have in place would it be at all possible to set the expected commit values for the branches once we converted the repository or what would happen?
| parameters={"model": git_repo_add_model}, | ||
| ) | ||
|
|
||
| # TODO Validate that the creation of the repository went as expected |
There was a problem hiding this comment.
I wonder if we should be adding more files under infrahub.core, at least we should have a discussion around it. I think we can keep these changes here until we have decided but it but we should have some consistent thought around where things are placed.
|
|
||
| assert repository.commit.value | ||
| assert repository.internal_status.value == "active" | ||
| # assert repository.operational_status.value == "online" --> it's unknown, is this an issue? |
There was a problem hiding this comment.
Is this an issue in the tests or something that we'd see in production as well?
There was a problem hiding this comment.
When we create a ReadOnlyRepository, fetch method is not called, which is responsible for setting the operational status, while it's the case for a read write Repository. I believe this may currently happen in production. I am not sure whether InfrahubReadOnlyRepository.sync_from_remote should call this fetch method or if this status should be set through another way.
LucasG0
force-pushed
the
lgu-convert-repos
branch
from
4bdb07f to
3cc29fa
Compare
This PR adds support for converting repository type, ie read-write to read-only and vice versa. It mainly contains some refactoring around the repository creation mutation so we can reuse this logic after converting an object.