store timestamp of last change for non-bug objects in the database

RESOLVED WONTFIX

Status

()

P5
enhancement
RESOLVED WONTFIX
10 years ago
9 years ago

People

(Reporter: Frank, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Details

Attachments

(1 attachment, 1 obsolete attachment)

12.47 KB, patch
mkanat
: review-
LpSolit
: review-
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
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

When you change an Object (Product, Flag,..) there is not creation/modification timestamp stored.

But for client this information is important for reduce the network bandwidth.

Actual I implement an example for change Mylyn to work with webservice to receive the configuration and I need this information.  

Reproducible: Always
We'll need a specific list of objects that you want the timestamp stored for, and then a separate bug for each one, blocking this bug.

I doubt you'd improve performance all that much, though--I'd imagine latency is more of a problem than bandwidth, and it's going to take the same latency to check timestamps as it will to download all the info about the objects.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: store timestamp of last change → store timestamp of last change for non-bug objects in the database
(Reporter)

Comment 2

10 years ago
(In reply to comment #1)
> We'll need a specific list of objects that you want the timestamp stored for,
> and then a separate bug for each one, blocking this bug.

I think that only one timestamp is needed that get updated if one configuration item get changed.

Items should be
- Classifications
- Products with subparts Component, versions and milestones
- Flags
- Custom Fields
- Bug Status Workflow
- Keywords
- Groups
- all tables listed in 'Field Values' administration

configuration item are all things that are in config.cgi

> 
> I doubt you'd improve performance all that much, though--I'd imagine latency is
> more of a problem than bandwidth, and it's going to take the same latency to
> check timestamps as it will to download all the info about the objects.

If you implement one webservice call that return all items from config.cgi then latency is not the important factor.

Currently I implement this extension for use with Mylyn in an separate package.
(Reporter)

Comment 3

9 years ago
Created attachment 401737 [details] [diff] [review]
patch, V1
Attachment #401737 - Flags: review?(mkanat)
Comment on attachment 401737 [details] [diff] [review]
patch, V1

Yeah, for now I'm not going to accept a patch that adds update_config_timestamp() to the CGIs directly. Instead you should be modifying Object->update and setting the timestamp of any object that has that column available (possibly by adding a new constant that specifies whether or not the timestamp column exists, or perhaps the name of the timestamp column) and then also adding an accessor to Bugzilla::Object called something like last_changed, that returns the change time (possibly as a DateTime object).
Attachment #401737 - Flags: review?(mkanat) → review-
(Reporter)

Comment 5

9 years ago
(In reply to comment #4)
> (From update of attachment 401737 [details] [diff] [review])
> Yeah, for now I'm not going to accept a patch that adds
> update_config_timestamp() to the CGIs directly. Instead you should be modifying
> Object->update and setting the timestamp of any object that has that column
> available (possibly by adding a new constant that specifies whether or not the
> timestamp column exists, or perhaps the name of the timestamp column) and then
> also adding an accessor to Bugzilla::Object called something like last_changed,
> that returns the change time (possibly as a DateTime object).

Sorry I did not know if I get this right. The files editflagtypes.cgi, editgroups.cgi, editkeywords.cgi, and editworkflow.cgi are icluded in the patch because they have hold the SQL statements insert, update and delete. To get this work I must move the SQL to the Bugzilla::Flagtype.pm, Bugzilla::Group.pm, Bugzilla::Keyword.pm and create an new Bugzilla::Status_workflow.pm. Is this correct?

The patch from file editvalues.cgi can be moved to Bugzilla::Field::Choice.

Do you want to add a column to every instance of an Object? By example add column last_changed to Flagtype and use SQL like "select max(last_changed) FROM Flagtype" to get the result.
editkeywords.cgi, at the least, definitely does not contain those SQL statements in modern versions of Bugzilla. Are you developing against HEAD?

You may have to wait until we move some of those things ourselves.

For now, just do one of them, whichever's easiest, and post that as a single patch to another bug blocking this one.
(Reporter)

Comment 7

9 years ago
(In reply to comment #6)
> editkeywords.cgi, at the least, definitely does not contain those SQL
> statements in modern versions of Bugzilla. Are you developing against HEAD?

Yes but in line 172 and 173 you find
    $dbh->do('DELETE FROM keywords WHERE keywordid = ?', undef, $keyword->id);
    $dbh->do('DELETE FROM keyworddefs WHERE id = ?', undef, $keyword->id);

> 
> You may have to wait until we move some of those things ourselves.
> 
> For now, just do one of them, whichever's easiest, and post that as a single
> patch to another bug blocking this one.

What way do you prefer?
a) one timestamp for all administration objects
b) one timestamp for every administration object class. Example: one timestamp for all product one for all components, ...
3) a timestamp for all administration object. Example separate timestamp for separate Products, ...

For me a) is enough. Maybe we need b) or c) later.
(Reporter)

Updated

9 years ago
Depends on: 523201
(Reporter)

Updated

9 years ago
Blocks: 504937
(Reporter)

Updated

9 years ago
Depends on: 523205
(Reporter)

Comment 8

9 years ago
Created attachment 407104 [details] [diff] [review]
patch, V

This patch use Object->insert_create_data, Object->update and Object->remove_from_db.
Objeckt use a new constant for do the config timestamp update.
Attachment #401737 - Attachment is obsolete: true
Attachment #407104 - Flags: review?(mkanat)

Updated

9 years ago
Priority: -- → P5

Comment 9

9 years ago
Comment on attachment 407104 [details] [diff] [review]
patch, V

>+use constant CONFIG_OBJECT => 'yes';

This should be a boolean 0/1. And the name is pretty meaningless, IMO.


>Index: Bugzilla/Config.pm

>+sub update_config_timestamp {
>+    my $dbh = Bugzilla->dbh;
>+    my $timestamp = $dbh->selectrow_array('SELECT NOW()');
>+    SetParam('config_lastupdate_timestamp', $timestamp);
>+    write_params();
>+}

A parameter can be manually edited. That's not a real parameter.


>Index: Bugzilla/Object.pm

>+    my $is_config_obj   = $self->CONFIG_OBJECT;
>+    if ($is_config_obj eq 'yes') {
>+        update_config_timestamp();
>+    }

This could simply be written as: update_config_timestamp() if $self->CONFIG_OBJECT.


>Index: Bugzilla/Config/Admin.pm

>+  {
>+   name => 'config_lastupdate_timestamp',
>+   type => 't',
>+   default => 'undefined'
>   });

It has no definition in admin/params/admin.html.tmpl.


That's just a quick review. mkanat probably has more things to say.
Attachment #407104 - Flags: review-
Yeah, generally what I have to say is that I don't want this--there should be a way to determine the last date the full configuration changed without having to resort to this sort of complexity.
(Reporter)

Comment 11

9 years ago
(In reply to comment #10)
> Yeah, generally what I have to say is that I don't want this--there should be a
> way to determine the last date the full configuration changed without having to
> resort to this sort of complexity.

An other way is to store the timestamp into every object in the database. But then we have a more expensive operation for retrieve the information.

If a client (like Mylyn) frequently asks if the configuration is actual. It is important that this operation is cheap. An max over all administration objects is much more expensive.

Thoughts?
Comment on attachment 407104 [details] [diff] [review]
patch, V

I don't want this to happen at all--too much complexity.
Attachment #407104 - Flags: review?(mkanat) → review-
I described another way of determining the last time the configuration changed in the bug about timestamping the config.cgi output.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → WONTFIX
(Reporter)

Comment 14

9 years ago
(In reply to comment #13)
> I described another way of determining the last time the configuration changed
> in the bug about timestamping the config.cgi output.

Did you mean adding a timestamp to the database (https://bugzilla.mozilla.org/show_bug.cgi?id=504612#c4)?
No, I meant the discussion we're having in bug 515644.
You need to log in before you can comment on or make changes to this bug.