Hi
I have found the cause of the problem with the update_repo_group API behavior
that I wrote about in my previous email.
The implementation of RepoGroupModel.update() does not seem to work correctly unless it includes a valid group_name in args,
even if the name is not changed.
If no new name is specified for the API parameter, it may be necessary to pass
the original name.
I have created a patch to api.py based on 088551a24485 from kallithea-imcoming.
Attachment: fix-update_repo_group-1.patch
Also, I thought it was not good to have inconsistent results depending on the call parameters, so I refactored the
implementation of RepoGroupModel.update().
It also includes changes that correct incorrect logging output. However, this
is not required for API operation.
Attachment: fix-update_repo_group-2.patch
Thanks
On 2022/10/30 21:00, toras wrote:
Hi
I still rely on translation tools.
Perhaps it is hard to read. I hope I can convey it properly.
I also tried attaching the patch, but I should add that I'm inexperienced with
python.
> Yes. Can you confirm this will solve the code problems you reported:
>
https://kallithea-scm.org/repos/kallithea-incoming/changeset/088551a24485247af328f0b8e395fdbbcd62a700
?
1. Modify create_repo()
I don't think this change is correct.
I overwrote api.py and called create_repo, but
enable_downloads/enable_statistics seems to have no effect.
I think the call flow during API execution is as follows.
api.py : ApiController.create_repo()
repo.py : RepoModel.create()
repo.py : (@celerylib.task) create_repo()
I think that the third function does not refer to form_data, but refers to
defs to obtain the default value and use it.
https://kallithea-scm.org/repos/kallithea/files/2dd317e9cc4b5ff94c1ddae7ee772d20b61259a9/kallithea/model/repo.py#L717
I think this is probably implemented in conjunction with the web front end.
I see two options for changes to the create_repo API.
I think this requires a policy decision.
Scenario 1:
The API will no longer accept that parameter.
Attachment: 1-scenario1.patch
Scenario 2:
Support parameter specification in RepoModel.
Attachment: 1-scenario2.patch
2. Modify update_repo_group()
I think there are two issues with parent handling.
The first is.
Other api parameters can basically target entities by name.
It seems to me that as the parent_group_id parameter to RepoGroupModel.update(), the API parameter should be passed with name
resolution instead of passing it as is.
Attachment: 2-parent_id.patch
The second is still unresolved and not yet known in detail.
Only information known at this time is provided.
The problem is that changing the parent group via the API will leave the
database and file system in an inconsistent state.
At that time, new_path does not seem to indicate the correct path in
RepoGroupModel.update(). So the folder is not moved.
It seems correct when run from the web front end. I still don't know what
the difference is.
Supplementation.
It may not be necessary, but I will publish the docker files and scripts that I
used in the confirmation work.
https://github.com/toras9000/kallithea_api_test
Thanks
# HG changeset patch
# User toras9000
# Date 1667574300 -32400
# Sat Nov 05 00:05:00 2022 +0900
# Branch fix-update_repo_group
# Node ID 05ed2d77c26da61ea41f08b7ab9d83078306805a
# Parent 088551a24485247af328f0b8e395fdbbcd62a700
fix: update_repo_group api
diff --git a/kallithea/controllers/api/api.py b/kallithea/controllers/api/api.py
--- a/kallithea/controllers/api/api.py
+++ b/kallithea/controllers/api/api.py
@@ -1791,11 +1791,20 @@
parent=None):
repo_group = get_repo_group_or_error(repogroupid)
+ if not group_name:
+ group_name = repo_group.name
+
+ parent_id = None
+ if parent is not None:
+ parent_group = get_repo_group_or_error(parent)
+ parent_id = parent_group.group_id
+
updates = {}
try:
store_update(updates, group_name, 'group_name')
store_update(updates, description, 'group_description')
- store_update(updates, parent, 'parent_group_id')
+ if parent_id is not None:
+ store_update(updates, parent_id, 'parent_group_id')
repo_group = RepoGroupModel().update(repo_group, updates)
meta.Session().commit()
return dict(
# HG changeset patch
# User toras9000
# Date 1667574446 -32400
# Sat Nov 05 00:07:26 2022 +0900
# Branch fix-update_repo_group
# Node ID 1f502ad271fd32733c9242d72faba5124fcaa310
# Parent 05ed2d77c26da61ea41f08b7ab9d83078306805a
fix: update_repo_group model
diff --git a/kallithea/model/repo_group.py b/kallithea/model/repo_group.py
--- a/kallithea/model/repo_group.py
+++ b/kallithea/model/repo_group.py
@@ -279,44 +279,46 @@
try:
repo_group = db.RepoGroup.guess_instance(repo_group)
old_path = repo_group.full_path
+ meta.Session().add(repo_group)
# change properties
if 'group_description' in repo_group_args:
repo_group.group_description =
repo_group_args['group_description']
if 'parent_group_id' in repo_group_args:
- repo_group.parent_group_id = repo_group_args['parent_group_id']
-
- if 'parent_group_id' in repo_group_args:
- assert repo_group_args['parent_group_id'] != '-1',
repo_group_args # RepoGroupForm should have converted to None
- repo_group.parent_group =
db.RepoGroup.get(repo_group_args['parent_group_id'])
+ parent_group_id = repo_group_args['parent_group_id']
+ assert parent_group_id != '-1', repo_group_args #
RepoGroupForm should have converted to None
+ repo_group.parent_group_id = parent_group_id
+ repo_group.parent_group = db.RepoGroup.get(parent_group_id)
if 'group_name' in repo_group_args:
group_name = repo_group_args['group_name']
if kallithea.lib.utils2.repo_name_slug(group_name) !=
group_name:
raise Exception('invalid repo group name %s' % group_name)
- repo_group.group_name = repo_group.get_new_name(group_name)
- new_path = repo_group.full_path
- meta.Session().add(repo_group)
+ new_name = repo_group.get_new_name(group_name)
+ log.debug(f'Fixing group {repo_group.group_name} to new name
{new_name}')
+ repo_group.group_name = new_name
- # iterate over all members of this groups and do fixes
+ # iterate over all members of this groups (including self) and do
fixes
# if obj is a repoGroup also fix the name of the group according
# to the parent
# if obj is a Repo fix it's name
# this can be potentially heavy operation
for obj in repo_group.recursive_groups_and_repos():
+ # Origin instance is already processed and skipped.
+ if obj is repo_group:
+ continue
# set the value from it's parent
if isinstance(obj, db.RepoGroup):
new_name = obj.get_new_name(obj.name)
- log.debug('Fixing group %s to new name %s'
- % (obj.group_name, new_name))
+ log.debug(f'Fixing group {obj.group_name} to new name
{new_name}')
obj.group_name = new_name
elif isinstance(obj, db.Repository):
# we need to get all repositories from this new group and
# rename them accordingly to new group path
new_name = obj.get_new_name(obj.just_name)
- log.debug('Fixing repo %s to new name %s'
- % (obj.repo_name, new_name))
+ log.debug(f'Fixing repo {obj.repo_name} to new name
{new_name}')
obj.repo_name = new_name
+ new_path = repo_group.full_path
self._rename_group(old_path, new_path)
return repo_group
_______________________________________________
kallithea-general mailing list
[email protected]
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
