Re: About API code comments and some behavior.

2022-12-29 Thread toras 

Hi

Thank you for your time and taking care of this.


On 2022/12/28 5:22, Mads Kiilerich wrote:

On 18/12/2022 10:48, toras wrote:

>> 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? 

...

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.



Ok. But I will just start out by making this piece of code use lazy %s 
expansion for logging like we do in all other places.


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.



Right - thanks for clarifying. I applied fix doing just that, but also made sure we don't do excessive logging when parameters 
specified without changing.



Thanks for the contribution.

/Mads


--
toras9000

___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: About API code comments and some behavior.

2022-12-27 Thread Mads Kiilerich 

On 18/12/2022 10:48, toras wrote:

>> 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? 

...

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.



Ok. But I will just start out by making this piece of code use lazy %s 
expansion for logging like we do in all other places.



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.



Right - thanks for clarifying. I applied fix doing just that, but also 
made sure we don't do excessive logging when parameters specified 
without changing.



Thanks for the contribution.

/Mads

___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: About API code comments and some behavior.

2022-12-18 Thread toras 

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

Re: About API code comments and some behavior.

2022-12-12 Thread Mads Kiilerich 

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

Re: About API code comments and some behavior.

2022-11-04 Thread toras 

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

Re: About API code comments and some behavior.

2022-10-30 Thread toras 

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
--
toras9000

On 2022/10/29 17:36, Mads Kiilerich wrote:

Hi

Yes. Can you confirm this will solve the code problems you reported: 
https://kallithea-scm.org/repos/kallithea-incoming/changeset/088551a24485247af328f0b8e395fdbbcd62a700 ?


/Mads


On 29/10/2022 09:28, toras wrote:

Hi

I then noticed that the enable_downloads/enable_statistics parameters in the 
create_repo API also had no effect.
It seems that no parameters are used and the repository's default settings are used. (I think this is probably the expected 
behavior.)

I think this one also needs to be removed from the API parameters and 
description.


Thanks



# HG changeset patch
# User toras9000
# Date 1667130294 -32400
#  Sun Oct 30 20:44:54 2022 +0900
# Branch create_repo_scenario1
# Node ID 6e77e0c3353c51e393b9fe70fc7ed4ce4887139c
# Parent  088551a24485247af328f0b8e395fdbbcd62a700
delete create_repo params

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
@@ -1133,8 +1133,6 @@
 repo_type=None, description='',
 private=False, clone_uri=None,
 landing_rev='rev:tip',
-enable_statistics=None,
-enable_downloads=None,
 copy_permissions=False):
 """
 Creates a repository. The repository name contains the full path, but 
the
@@ -1158,10 +1156,6 @@
 :type clone_uri: str
 :param landing_rev: :
 :type landing_rev: str
-:param enable_downloads:
-:type enable_downloads: bool
-:param enable_statistics:
-:type enable_statistics: bool
 :param copy_permissions: Copy permission from group that repository is
 being created.
 :type copy_permissions: bool
@@ -1215,10 +1209,6 @@
 private = defs.get('repo_private') or False
 if repo_type is None:
 repo_type = defs.get('repo_type')
-if enable_statistics is None:
-enable_statistics = defs.get('repo_enable_statistics')
-if enable_downloads is None:
-enable_downloads = defs.get('repo_enable_downloads')
 
 try:
 data = dict(
@@ -1230,8 +1220,6 @@
 clone_uri=clone_uri,
 repo_group=group_name,
 repo_landing_rev=landing_rev,
-repo_enable_statistics=enable_statistics,
-repo_enable_downloads=enable_downloads,
 repo_copy_permissions=copy_permissions,
 )
 
# HG changeset patch
#

Re: About API code comments and some behavior.

2022-10-29 Thread Mads Kiilerich 

Hi

Yes. Can you confirm this will solve the code problems you reported: 
https://kallithea-scm.org/repos/kallithea-incoming/changeset/088551a24485247af328f0b8e395fdbbcd62a700 
?


/Mads


On 29/10/2022 09:28, toras wrote:

Hi

I then noticed that the enable_downloads/enable_statistics parameters 
in the create_repo API also had no effect.
It seems that no parameters are used and the repository's default 
settings are used. (I think this is probably the expected behavior.)
I think this one also needs to be removed from the API parameters and 
description.



Thanks



___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: About API code comments and some behavior.

2022-10-29 Thread toras 

Hi

I then noticed that the enable_downloads/enable_statistics parameters in the 
create_repo API also had no effect.
It seems that no parameters are used and the repository's default settings are used. (I think this is probably the expected 
behavior.)

I think this one also needs to be removed from the API parameters and 
description.


Thanks

--
toras9000

On 2022/10/15 12:31, toras wrote:

Hi


 > (If you are working from a repo checkout, it would be more helpful if you 
could send a diff.)

You are right. I wasn't used to it.
I will do so on future occasions.

 >> +    "gist": 
 > What is gist object? That should perhaps be clarified. Or perhaps it is a 
bug that it is returned ...

I wrote it in the sense that the  in the result of this 'create_gist' is JSON with the same structure as the result 
of 'get_gist'.

It's information about the created gist and I think it's the correct return 
value.
To avoid misinterpretation as an empty object, I imitated 'create_user' comment 
and wrote it like this.

Specifically,  was such JSON data.
   "gist": {"gist_id": 1, "type": "public", "access_id": "1", "description": "", "url": "http://localhost:/_admin/gists/1";, 
"expires": -1.0, "created_on": "2022-10-14T14:20:24.625"}



 >> - The 'parent' parameter of 'update_repo_group' does not work.
 > A quick look:  The update_repo_group API arguments seems to be handled by
 > 
https://kallithea-scm.org/repos/kallithea/files/7037365a/kallithea/model/repo_group.py#L278
 . So perhaps the code in api.py
 > should pass it as 'parent_group_id' instead of 'parent_group'?
 > (But also, 'owner' doesn't seem to be handled at all. Does owner change 
really work for you? But also, I don't think it matters
 > much who owns a repo group. Admin rights does the same thing. So repo group 
owner should perhaps just be removed from api.py and
 > documentation?)

I'm not confident in my reading comprehension, but...

As you say, it looks like it would be necessary to pass it by 'parent_group_id'.
Also, is it necessary to not pass the 'parent_group_id' if it is not moved?
And I think that api.py should also resolve with get_repo_group_or_error() so 
that both name and id values are available.

And 'owner' certainly doesn't work either. I hadn't noticed.
Looking at the comments on RepoGroupModel.create(), did the implementer plan to 
use owner in the future?
There is also a countermeasure to make it changeable with update, right?
But for consistency in the current situation, it seems reasonable to remove it 
from the API parameters and documentation.


 > Anyway:
 > I also pushed some further cleanup of the documentation in api.py and 
api.rst .That's on the *stable* branch. It would be great
 > if they could converge, and we could generate api.rst from api.py .
 > If you want to improve documentation further, take a look at 
https://kallithea-scm.org/repos/kallithea/pull-request/325/_/api
 > and propose api.py changes to make a grand unification.

Thank you. But as of now I do not have any further changes.



Thanks



___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: About API code comments and some behavior.

2022-10-14 Thread toras 

Hi


> (If you are working from a repo checkout, it would be more helpful if you 
could send a diff.)

You are right. I wasn't used to it.
I will do so on future occasions.

>> +"gist": 
> What is gist object? That should perhaps be clarified. Or perhaps it is a bug 
that it is returned ...

I wrote it in the sense that the  in the result of this 'create_gist' is JSON with the same structure as the result 
of 'get_gist'.

It's information about the created gist and I think it's the correct return 
value.
To avoid misinterpretation as an empty object, I imitated 'create_user' comment 
and wrote it like this.

Specifically,  was such JSON data.
  "gist": {"gist_id": 1, "type": "public", "access_id": "1", "description": "", "url": "http://localhost:/_admin/gists/1";, 
"expires": -1.0, "created_on": "2022-10-14T14:20:24.625"}



>> - The 'parent' parameter of 'update_repo_group' does not work.
> A quick look:  The update_repo_group API arguments seems to be handled by
> 
https://kallithea-scm.org/repos/kallithea/files/7037365a/kallithea/model/repo_group.py#L278
 . So perhaps the code in api.py
> should pass it as 'parent_group_id' instead of 'parent_group'?
> (But also, 'owner' doesn't seem to be handled at all. Does owner change 
really work for you? But also, I don't think it matters
> much who owns a repo group. Admin rights does the same thing. So repo group 
owner should perhaps just be removed from api.py and
> documentation?)

I'm not confident in my reading comprehension, but...

As you say, it looks like it would be necessary to pass it by 'parent_group_id'.
Also, is it necessary to not pass the 'parent_group_id' if it is not moved?
And I think that api.py should also resolve with get_repo_group_or_error() so 
that both name and id values are available.

And 'owner' certainly doesn't work either. I hadn't noticed.
Looking at the comments on RepoGroupModel.create(), did the implementer plan to 
use owner in the future?
There is also a countermeasure to make it changeable with update, right?
But for consistency in the current situation, it seems reasonable to remove it 
from the API parameters and documentation.


> Anyway:
> I also pushed some further cleanup of the documentation in api.py and api.rst 
.That's on the *stable* branch. It would be great
> if they could converge, and we could generate api.rst from api.py .
> If you want to improve documentation further, take a look at 
https://kallithea-scm.org/repos/kallithea/pull-request/325/_/api
> and propose api.py changes to make a grand unification.

Thank you. But as of now I do not have any further changes.



Thanks

--
toras9000

On 2022/10/14 21:51, Mads Kiilerich wrote:

Hi

Thank you. (If you are working from a repo checkout, it would be more helpful if you could send a diff.) I haven't verified in 
detail, but all the changes seem plausible ;-) I have pushed them to the stable branch.


One question though:


   id : 
   result : {
 "msg": "created new gist",
-    "gist": {}
+    "gist": 
   }
   error :  null


What is gist object? That should perhaps be clarified. Or perhaps it is a bug 
that it is returned ...


- The 'parent' parameter of 'update_repo_group' does not work.
  It appears to accept the 'parent' parameter, but specifying it seems to have no effect. 


A quick look:  The update_repo_group API arguments seems to be handled by 
https://kallithea-scm.org/repos/kallithea/files/7037365a/kallithea/model/repo_group.py#L278 . So perhaps the code in api.py 
should pass it as 'parent_group_id' instead of 'parent_group'?
(But also, 'owner' doesn't seem to be handled at all. Does owner change really work for you? But also, I don't think it matters 
much who owns a repo group. Admin rights does the same thing. So repo group owner should perhaps just be removed from api.py and 
documentation?)


Anyway:
I also pushed some further cleanup of the documentation in api.py and api.rst .That's on the *stable* branch. It would be great 
if they could converge, and we could generate api.rst from api.py .
If you want to improve documentation further, take a look at https://kallithea-scm.org/repos/kallithea/pull-request/325/_/api 
and propose api.py changes to make a grand unification.


/Mads


On 13/10/2022 15:00, toras wrote:

Hi.


There was an API-related topic a little while ago,so there may be things that are currently being changed, but I recently 
noticed something while writing a client that uses the API, so I'd like to report it.


- The comments in api.py have some differences from the implementation.
  I tried calling the API and attached a file that changed the part that is 
different from the result.
  (Based on commit 7037365a7bc3.)
  I don't know if this is necessary, but for your information.

- The 'parent' parameter of 'update_repo_group' does not work.
  It appears to accept the 'parent' parameter, but specifying it seems to have 
no effect.


Thanks

Re: About API code comments and some behavior.

2022-10-14 Thread Mads Kiilerich 

Hi

Thank you. (If you are working from a repo checkout, it would be more 
helpful if you could send a diff.) I haven't verified in detail, but all 
the changes seem plausible ;-) I have pushed them to the stable branch.


One question though:


   id : 
   result : {
 "msg": "created new gist",
-    "gist": {}
+    "gist": 
   }
   error :  null


What is gist object? That should perhaps be clarified. Or perhaps it is 
a bug that it is returned ...



- The 'parent' parameter of 'update_repo_group' does not work.
  It appears to accept the 'parent' parameter, but specifying it seems 
to have no effect. 


A quick look:  The update_repo_group API arguments seems to be handled 
by 
https://kallithea-scm.org/repos/kallithea/files/7037365a/kallithea/model/repo_group.py#L278 
. So perhaps the code in api.py should pass it as 'parent_group_id' 
instead of 'parent_group'?
(But also, 'owner' doesn't seem to be handled at all. Does owner change 
really work for you? But also, I don't think it matters much who owns a 
repo group. Admin rights does the same thing. So repo group owner should 
perhaps just be removed from api.py and documentation?)


Anyway:
I also pushed some further cleanup of the documentation in api.py and 
api.rst .That's on the *stable* branch. It would be great if they could 
converge, and we could generate api.rst from api.py .
If you want to improve documentation further, take a look at 
https://kallithea-scm.org/repos/kallithea/pull-request/325/_/api and 
propose api.py changes to make a grand unification.


/Mads


On 13/10/2022 15:00, toras wrote:

Hi.


There was an API-related topic a little while ago,so there may be 
things that are currently being changed, but I recently noticed 
something while writing a client that uses the API, so I'd like to 
report it.


- The comments in api.py have some differences from the implementation.
  I tried calling the API and attached a file that changed the part 
that is different from the result.

  (Based on commit 7037365a7bc3.)
  I don't know if this is necessary, but for your information.

- The 'parent' parameter of 'update_repo_group' does not work.
  It appears to accept the 'parent' parameter, but specifying it seems 
to have no effect.



Thanks

--
toras9000


___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general



___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general

About API code comments and some behavior.

2022-10-13 Thread toras 

Hi.


There was an API-related topic a little while ago,so there may be things that are currently being changed, but I recently 
noticed something while writing a client that uses the API, so I'd like to report it.


- The comments in api.py have some differences from the implementation.
  I tried calling the API and attached a file that changed the part that is 
different from the result.
  (Based on commit 7037365a7bc3.)
  I don't know if this is necessary, but for your information.

- The 'parent' parameter of 'update_repo_group' does not work.
  It appears to accept the 'parent' parameter, but specifying it seems to have 
no effect.


Thanks

--
toras9000

# -*- coding: utf-8 -*-
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see .
"""
kallithea.controllers.api.api
~

API controller for Kallithea

This file was forked by the Kallithea project in July 2014.
Original author and date, and relevant copyright and licensing information is 
below:
:created_on: Aug 20, 2011
:author: marcink
:copyright: (c) 2013 RhodeCode GmbH, and others.
:license: GPLv3, see LICENSE.md for more details.
"""

import logging
import traceback
from datetime import datetime

from tg import request

from kallithea.controllers.api import JSONRPCController, JSONRPCError
from kallithea.lib.auth import (AuthUser, HasPermissionAny, 
HasPermissionAnyDecorator, HasRepoGroupPermissionLevel, HasRepoPermissionLevel,
HasUserGroupPermissionLevel)
from kallithea.lib.exceptions import DefaultUserException, 
UserGroupsAssignedException
from kallithea.lib.utils import repo2db_mapper
from kallithea.lib.vcs.backends.base import EmptyChangeset
from kallithea.lib.vcs.exceptions import EmptyRepositoryError
from kallithea.model import db, meta, userlog
from kallithea.model.changeset_status import ChangesetStatusModel
from kallithea.model.comment import ChangesetCommentsModel
from kallithea.model.gist import GistModel
from kallithea.model.pull_request import PullRequestModel
from kallithea.model.repo import RepoModel
from kallithea.model.repo_group import RepoGroupModel
from kallithea.model.scm import ScmModel, UserGroupList
from kallithea.model.user import UserModel
from kallithea.model.user_group import UserGroupModel


log = logging.getLogger(__name__)


def store_update(updates, attr, name):
"""
Stores param in updates dict if it's not None (i.e. if user explicitly set
a parameter). This allows easy updates of passed in params.
"""
if attr is not None:
updates[name] = attr


def get_user_or_error(userid):
"""
Get user by id or name or return JsonRPCError if not found

:param userid:
"""
user = UserModel().get_user(userid)
if user is None:
raise JSONRPCError("user `%s` does not exist" % (userid,))
return user


def get_repo_or_error(repoid):
"""
Get repo by id or name or return JsonRPCError if not found

:param repoid:
"""
repo = RepoModel().get_repo(repoid)
if repo is None:
raise JSONRPCError('repository `%s` does not exist' % (repoid,))
return repo


def get_repo_group_or_error(repogroupid):
"""
Get repo group by id or name or return JsonRPCError if not found

:param repogroupid:
"""
repo_group = db.RepoGroup.guess_instance(repogroupid)
if repo_group is None:
raise JSONRPCError(
'repository group `%s` does not exist' % (repogroupid,))
return repo_group


def get_user_group_or_error(usergroupid):
"""
Get user group by id or name or return JsonRPCError if not found

:param usergroupid:
"""
user_group = UserGroupModel().get_group(usergroupid)
if user_group is None:
raise JSONRPCError('user group `%s` does not exist' % (usergroupid,))
return user_group


def get_perm_or_error(permid, prefix=None):
"""
Get permission by id or name or return JsonRPCError if not found

:param permid:
"""
perm = db.Permission.get_by_key(permid)
if perm is None:
raise JSONRPCError('permission `%s` does not exist' % (permid,))
if prefix:
if not perm.permission_name.startswith(prefix):
raise JSONRPCError('permission `%s` is invalid, '
   'should start with %s' % (permid, prefix))
return perm


def get_gist_or_error(gistid):
"""
Get gist by id or gist_access_id or return Js