Hi
Thank you for confirming the details of the content.
>> 1. Modify create_repo()
>> ...
>
> Right. Thanks. I will go with scenario2 ... but take it further and make
repo_enable_downloads/repo_enable_statistics mandatory
> in the internal API and expose them in the UI form as well. As a part of
that, I will also add better test coverage and clean up
> the UI forms.
That is the most preferable way to handle it. Thank you.
>> Attachment: fix-update_repo_group-2.patch
>
> Lots of things happening in that patch. Can you explain in more details what
is changed and why ... and perhaps do it in several
> simpler changesets?
Sorry, I didn't explain myself well enough.
The change had two purposes.
The first is that RepoGroupModel.update works without specifying a group_name.
This has already been addressed in Rev.8732.
As for repo_group.parent_group and repo_group.parent_group_id, I was guessing what you pointed out, but wasn't sure, so I left
what I had done in the original code.
The second is to modify the log output.
Sorry for the confusion, the change to the f string was just for my own viewing
pleasure, not related to the issue.
The entities enumerated by recursive_groups_and_repos() to update the full path of the child elements are processed with log
output, but the starting repository group has already had its entities updated before the loop.
So, for example, updating the name from 'GroupA' to 'GroupB' will result in the
following log output
(Only the origin entity. Child elements are correct logs.)
[kallithea.model.repo_group] Fixing group GroupB to new name GroupB
The following logs should be correct.
[kallithea.model.repo_group] Fixing group GroupA to new name GroupB
For the purpose of logging the name before the change, the patch outputs the log before updating the entity and skips the
originating entity in a loop process.
Thanks
--
toras9000
On 2022/12/13 5:25, Mads Kiilerich wrote:
On 30/10/2022 13:00, toras wrote:
> 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
Right. Thanks. I will go with scenario2 ... but take it further and make repo_enable_downloads/repo_enable_statistics mandatory
in the internal API and expose them in the UI form as well. As a part of that, I will also add better test coverage and clean up
the UI forms.
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
Agreed. I will do that ... but with slight changes.
The second is still unresolved and not yet known in detail.
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.
...
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
Agreed, but I think the problem is in RepoGroupModel.update , where it failed to update group_name to the new full path path
after moving *if* group_name wasn't specified. I will fix that instead.
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
Lots of things happening in that patch. Can you explain in more details what is changed and why ... and perhaps do it in several
simpler changesets?
For example:
What inconsistent results did you see?
What logging was incorrect? (But to avoid pointless formatting of log strings that will be ignored because of the logging level,
we generally prefer to avoid using log.debug('%s' % x) but prefer log.debug('%s', x) .)
Also note that repo_group.parent_group is a convenience thing in the Object Relationship Mapper to automatically fetch whatever
repo_group.parent_group_id refers to. When one of them are changed, the other one should change too.
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
Thank you. There are so many ways to do Docker. Great that your setup can cover this and provide additional testing compared to
what is in the upstream repo.
I took a bit of a chance and pushed the changes to the stable branch. I hope
you can confirm it works for you now.
/Mads
_______________________________________________
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
