About API code comments and some behavior.
toras
toras9000 at gmail.com
Fri Nov 4 15:38:15 UTC 2022
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
-------------- next part --------------
# 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(
-------------- next part --------------
# 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
More information about the kallithea-general
mailing list