504937 - WebService function to get global configuration information about Bugzilla (Bugzilla.config)

Status: NEW

(Bugzilla :: WebService, enhancement)

Description

[Frank Becker]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_7; de-de) AppleWebKit/530.19.2 (KHTML, like Gecko) Version/4.0.2 Safari/530.19
Build Identifier: 3.4.rc1

Currently forClients like Mylyn there is only a way to receive the configuration using HTTP.
In HTTP you get not all the needed information, so I start to implement an new webservice package for this.

Hope that I can post a first version soon.
 

Reproducible: Always

Comment 1

[Frédéric Buclin]

That's a bit vague. Which configuration are you talking about?

Comment 2

[Frank Becker]

(In reply to comment #1)
> That's a bit vague. Which configuration are you talking about?

I plan to replace the HTTP request .../config.cgi?ctype=rdf with one webservice call

Comment 3

[Max Kanat-Alexander]

I don't think we want all the information from config.cgi in one webservice call. In any case, you'd better just start with a small portion of it, or the patch will be too large to review.

And I suspect the function should be Bugzilla.config.

Comment 4

[Frank Becker]

Attachment: POC V1

Here is the Perl code i implement.

Please keep in mind that I'm not an Perl expert.

If I should add comments please give me a pointer where I can find information how to generate the POD.

Comment 5

[Max Kanat-Alexander]

Frank: If you'd like  a review, you'll have to follow our development process, listed here:

  http://wiki.mozilla.org/Bugzilla:Developers

Comment 6

[Frank Becker]

Attachment: patch, V1

(In reply to comment #5)
> Frank: If you'd like  a review, you'll have to follow our development process,
> listed here:
> 
>   http://wiki.mozilla.org/Bugzilla:Developers

Hopefully I have followed the necessary things.

Comment 7

[Max Kanat-Alexander]

Comment on attachment 389385 [details] [diff] [review]
patch, V1

You shouldn't be adding a new file. If you want to add information about products, that should go into Product.pm. There is already a Product.get, in fact.

If you want to add global information, that should go into Bugzilla.pm.

There already is a Bug.legal_values, so you don't have to reimplement that.

Function names should be named_like_this, not namedLikeThis (see the "perlstyle" man page).

You should file a separate bug for each function that you want to add.

Comment 8

[David Lawrence [:dkl]]

Attachment: 504937_1.patch

Comment 9

[:glob ✱]

Comment on attachment 8596110 [details] [diff] [review]
504937_1.patch

Review of attachment 8596110 [details] [diff] [review]:
-----------------------------------------------------------------

that's a very heavy call for data that is reasonable static; on large / high-traffic sites that may be problematic.

this is a prime candidate for storing in memcache.  check that each of the parts of this response have IS_CONFIG set to true in their package.  if not, file blocking bugs.  use memcached's set/get_config to ensure the cache is invalidated when the configuration is updated.

as there are some user specific parts of this response, all list of the user's group ids will have to be part of the cache key.

::: Bugzilla/API/1_0/Resource/Bugzilla.pm
@@ +242,5 @@
> +    $vars->{'platform'}   = get_legal_field_values('rep_platform');
> +    $vars->{'op_sys'}     = get_legal_field_values('op_sys');
> +    $vars->{'keyword'}    = [ Bugzilla::Keyword->get_all ];
> +    $vars->{'resolution'} = get_legal_field_values('resolution');
> +    $vars->{'status'}     = get_legal_field_values('bug_status');

nit: you could probably just loop over the fieldnames to avoid repetition

@@ +244,5 @@
> +    $vars->{'keyword'}    = [ Bugzilla::Keyword->get_all ];
> +    $vars->{'resolution'} = get_legal_field_values('resolution');
> +    $vars->{'status'}     = get_legal_field_values('bug_status');
> +    $vars->{'custom_fields'} =
> +        [ grep {$_->is_select} Bugzilla->active_custom_fields ];

nit: no need to wrap this line

@@ +247,5 @@
> +    $vars->{'custom_fields'} =
> +        [ grep {$_->is_select} Bugzilla->active_custom_fields ];
> +
> +    # Include a list of product objects.
> +    if ($params->{product}) {

the 'product' parameter isn't documented

@@ +252,5 @@
> +        my @products = ref $params->{product}
> +                       ? @$params->{product}
> +                       : ($params->{product});
> +        foreach my $product_name (@products) {
> +            my $product = new Bugzilla::Product({ name => $product_name });

don't use old style perl constructors, and use the cache:
  = Bugzilla::Product->new({ name => $product_name, cache => 1 })

@@ +657,5 @@
> +
> +=item B<Params>
> +
> +You can pass the optional parameter C<flags> set to 0 to disable
> +returning of detailed flag information which may increase performance.

we should default to doing less work, and let consumers opt-in to doing more.
please invert this param

::: template/en/default/config.json.tmpl
@@ +3,5 @@
> +  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +  #
> +  # This Source Code Form is "Incompatible With Secondary Licenses", as
> +  # defined by the Mozilla Public License, v. 2.0.
> +  #%]

this is a very slow and error-prone way to generate json.
instead generate the json in perl, return it directly.
we shouldn't need to use a template to do this.

Comment 10

[David Lawrence [:dkl]]

(In reply to Byron Jones ‹:glob› from comment #9)
> ::: template/en/default/config.json.tmpl
> @@ +3,5 @@
> > +  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
> > +  #
> > +  # This Source Code Form is "Incompatible With Secondary Licenses", as
> > +  # defined by the Mozilla Public License, v. 2.0.
> > +  #%]
> 
> this is a very slow and error-prone way to generate json.
> instead generate the json in perl, return it directly.
> we shouldn't need to use a template to do this.

The primary reason for this was so that you could also do config.cgi?ctype=json similar to rdf and js. But, agreed that since this is a new feature, we can skip the config template and enforce coming from the REST API only.

dkl

Comment 12

[Jeff Griffiths (:canuckistani) (:⚡︎)]

So - I'm apparently the maintainer of bz.js[1] and have requests from Wes related to bugherder to support /configuration somehow. Currently I'm special-casing the call back to /bzapi to help people targeting mozilla's bugzilla instance, but it's obv not great for anyone else using Bugzilla who wants a node/browser compatible client library.

Comment 13

[Kohei Yoshino [:kohei]]

My BzDeck app is also using BzAPI only for the configuration method. It would be great to have this in the standard REST API!

Comment 14

[David Lawrence [:dkl]]

(In reply to Jeff Griffiths (:canuckistani) from comment #12)
> So - I'm apparently the maintainer of bz.js[1] and have requests from Wes
> related to bugherder to support /configuration somehow. Currently I'm
> special-casing the call back to /bzapi to help people targeting mozilla's
> bugzilla instance, but it's obv not great for anyone else using Bugzilla who
> wants a node/browser compatible client library.

(In reply to Kohei Yoshino [:kohei] from comment #13)
> My BzDeck app is also using BzAPI only for the configuration method. It
> would be great to have this in the standard REST API!

While I am going to revise this, it would be useful feedback if you could give me a list of the most needed information to help your apps. Using the current /bzapi/configuration as a reference, what attributes do you use most often, never use, or would like to see added? Should I include all of the information currently there? 

Thanks!
dkl

Comment 15

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to David Lawrence [:dkl] from comment #14)
> While I am going to revise this, it would be useful feedback if you could
> give me a list of the most needed information to help your apps. Using the
> current /bzapi/configuration as a reference, what attributes do you use most
> often, never use, or would like to see added? Should I include all of the
> information currently there? 

Wes is driving this requirement currently for bugherder so passing the buck to him.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Bugherder's use of configuration data is in https://github.com/mozilla/Bugherder/blob/master/js/ConfigurationData.js#L43

So at a minimum, the flag_type object and the product object (and at least the target_milestone_detail array within each product).

Comment 17

[Kohei Yoshino [:kohei]]

BzDeck is using the max_attachment_size, classification, product and field configurations so far.

https://wiki.mozilla.org/Bugzilla:BzAPI:Objects:Configuration#Configuration
https://github.com/bzdeck/bzdeck/search?q=%22server.data.config%22

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Out of curiosity, would it be outside the realm of possibility for a call to /rest/configuration (or whatever it ends up being) to return the full configuration object (providing everything bzapi/configuration, though possibly differently organized), but then also have /rest/configuration?properties=[List of Properties] that could return a subset of the configuration?

So if I'm only interested in flag_type and product configuration, I could call /rest/configuration?properties=flag_type,product

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

ni?dkl for the responses and questions up there ^

Comment 20

[David Lawrence [:dkl]]

(In reply to Wes Kocher (:KWierso) from comment #18)
> Out of curiosity, would it be outside the realm of possibility for a call to
> /rest/configuration (or whatever it ends up being) to return the full
> configuration object (providing everything bzapi/configuration, though
> possibly differently organized), but then also have
> /rest/configuration?properties=[List of Properties] that could return a
> subset of the configuration?
> 
> So if I'm only interested in flag_type and product configuration, I could
> call /rest/configuration?properties=flag_type,product

It is definitely possible and should be a part of the next patch revision. I will make it like other API objects in that it honors the include_fields and exclude_fields parameters. This should help in the case of the some of the attributes take time to load and should not load if the user is not asking for them. Memcache will also help in performance.

dkl

Comment 21

[Kohei Yoshino [:kohei]]

I was surprised to realize that the current API doesn't have the keyword list. Any ETA here?

Comment 22

[David Lawrence [:dkl]]

(In reply to Kohei Yoshino [:kohei] from comment #21)
> I was surprised to realize that the current API doesn't have the keyword
> list. Any ETA here?

Has been low priority for a bit but I am trying to clean up some of my old bugs so I will see if I can get a new patch up for this in the next day or so with all of the suggested changes.

dkl

Comment 23

[Dennis Roczek]

I believe that bug is trying to do what I wanted to propose: I was checking
https://wiki.mozilla.org/Bugzilla:BzAPI:Objects:Configuration#Classification
but sadly it doesn't provide any possibility to get the "terms" defined by administrators. ( https://github.com/mozilla/webtools-bmo-bugzilla/blob/1f1f0d3276bef3844e8d381ba8277585c671466e/template/en/default/global/variables.none.tmpl )

The reason for that request is that I'm helping to developing the mediawiki-bugzilla-integration extension ( https://github.com/mozilla/mediawiki-bugzilla/ ) and there is an open pull request at GitHub ( https://github.com/mozilla/mediawiki-bugzilla/commit/193b35813d9403ba544be4cd3eb9daf77dbade89 ) which is not merged because "we" either have to specify our own generic message or (what I do prefer) use the same message as Bugzilla would display...