pullrequest creation API

[Thomas De Schampheleire]

Hi Mads,

On Thu, Mar 12, 2015 at 3:08 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 03/11/2015 09:56 PM, Thomas De Schampheleire wrote:
>>
>> Hi,
>>
>> I would like to create a way to create a pull request
>> programmatically, through an API call.
>
>
> You also asked for feedback on the related
> https://bitbucket.org/conservancy/kallithea/pull-request/47/add-pullrequest-json-page/diff
> . Would that PR solve your problem?

No, it's not the same objective.
Pull request #47 creates an API to obtain information about a pull
request. It is read-only.
What I want to achieve is the _creation_ of a new pull request.

Clearly both are related, but not the same.

>
>> At first sight, using the api_access=True method to create a callable
>> web view is the easiest, since you could take in the request and pass
>> it to the existing PullrequestsController:create() method. However,
>> this existing create method is decorated with @NotAnonymous, causing
>> it to be unavailable for API calls.
>>
>> The alternative is to create an API method in controllers/api/api.py,
>> but it's unclear to me how to hook this into the
>> PullrequestsController.
>
>
> I think that is because we have too much logic in the controller. I think
> that in some MVC universes, the controller should only deal with the view
> specific code (html page and user interaction). The logic we want to expose
> for general purposes should thus not be view specific but should be a part
> of the model.

The way I understand the MVC concept:
- the model is an encapsulation of the database
- the view is the representation towards the user
- the controller is the engine between both, taking data from the
model and presenting it to the user, or taking input from the user and
putting it in the model.

When representing information about a pullrequest, as pull request #47
is doing, one could argue that it could be retrieved from the model.
But when trying to create a new pullrequest, as I am trying to
accomplish, the model is not the right place I think. The parsing of
the input and the writing of the data in the model is the
responsibility of the controller, I think.

>
>> How to proceed here?
>> Should I remove the NotAnonymous decorator and pass the requested user
>> as argument to the API call?
>
>
> I am not sure in what way NotAnonymous and LoginRequired really are
> different. For example, it seems weird that NotAnonymous doesn't consider
> the IP filtering. We should perhaps use LoginRequired everywhere? Or would
> this checking have too much overhead? I don't know. Try!

I'm also not sure what the purpose of NotAnonymous is. It boils down
to the purpose of the default user: why do we need it?
If you're not logged in, does Kallithea treat you as the default user?

Why would it be too much overhead? Just to check the IP against a list
of IPs or subnets?

Moreover, all cases where NotAnonymous is used also use the
LoginRequired decorator, with the exception of
ReposController:create and ReposController:create_repository.

(grep -rn -C1 NotAnonymous kallithea/ )

>
>> I was also a bit confused on how these API keys are working: it seems
>> that whenever you call an API, you are really the default (anonymous)
>> user, regardless of the API key you are using.
>> For this use case, it would be better to determine the user this API
>> key is belonging to.
>
>
> As far as I can see, BaseController.__call__ in kallithea/lib/base.py _will_
> determine the user based on the api key.
>
> LoginRequired will consider the presence of an api key as evidence that the
> user was authenticated with that key.

In my testing I made a mistake in the API key. With the correct key,
BaseController.__call__ indeed uses the API key to authenticate the
right user.

Thanks,
Thomas