Modify

Opened 7 months ago

Closed 6 months ago

#11553 closed defect (fixed)

Can't close a project?

Reported by: endquote Owned by: falkb
Priority: normal Component: SimpleMultiProjectPlugin
Severity: normal Keywords:
Cc: Trac Release: 1.0

Description

If I check the "closed" box on a project and then submit the form, I'm taken back to the projects list -- but no projects are shown! Scary! If I reload it, they come back, but the project I tried to close is still not closed.

Attachments (2)

patchedFiles.zip (4.8 KB) - added by falkb 7 months ago.
patchedFiles.2.zip (4.8 KB) - added by falkb 6 months ago.
second version

Download all attachments as: .zip

Change History (47)

comment:1 Changed 7 months ago by falkb

This is strange. I saw such misbehaviour before my big svn commit of model.py repairing the db access routines, but I think you're already on the latest revision. The reason was that db writing did not work because of some mistakes in model.py. Uhmm... all projects I've closed lately didn't show that bug anymore. Can you see something in trac.log?

comment:2 Changed 7 months ago by endquote

I removed and reinstalled the plugin from the latest source and the behavior remains.

I did see this in the log following the pressing of the "modify" button after checking the "closed" box.

2014-02-09 12:47:41,808 Trac[admin] INFO: Simple Multi Project: Modify project SST
2014-02-09 12:47:41,823 Trac[admin] ERROR: Modify Project Error: integer out of range
2014-02-09 12:47:41,823 Trac[admin] ERROR: SimpleMultiProject Error: Failed to added project 'SST'

comment:3 Changed 7 months ago by anonymous

What is your db type (SQLite, MySQL, ...)?

comment:4 Changed 7 months ago by endquote

postgres 9.3.

comment:5 Changed 7 months ago by anonymous

I wonder if function update_project() has that exception in self.__SmpModel.update_project(). Could you put self.log.info("%s modified." % (name)) after self.__SmpModel.update_project() and tell me if this logging appears in trac.log then?

comment:6 Changed 7 months ago by anonymous

should be line 65 in admin.py

comment:7 Changed 7 months ago by endquote

I downloaded the zipped source again, added that line to admin.py, re-compiled and re-installed the egg, restarted trac, and tried to close the project again.

2014-02-10 09:55:46,335 Trac[main] DEBUG: Dispatching <RequestWithSession "POST '/admin/projects/simplemultiproject/5'">
2014-02-10 09:55:46,335 Trac[webui] DEBUG: T&E matched: /admin/projects/simplemultiproject/5  None
2014-02-10 09:55:46,338 Trac[session] DEBUG: Retrieving session for ID u'josh'
2014-02-10 09:55:46,348 Trac[admin] INFO: Simple Multi Project: Modify project SST
2014-02-10 09:55:46,349 Trac[admin] ERROR: Modify Project Error: integer out of range
2014-02-10 09:55:46,349 Trac[admin] ERROR: SimpleMultiProject Error: Failed to added project 'SST'
2014-02-10 09:55:46,349 Trac[chrome] DEBUG: Prepare chrome data for request

So the new log line is not appearing, which does seem to indicate an exception in self.__SmpModel.update_project().

comment:8 Changed 7 months ago by rjollos

You might want to try enabling SQL debugging. See TracIni#trac-section: [trac] debug_sql.

comment:9 Changed 7 months ago by endquote

It looks like the closed field is being set to 1392068154000000L, which I'm guessing isn't what postgres wants.

Log with debug_sql on.

2014-02-10 13:36:10,612 Trac[session] DEBUG: Retrieving session for ID u'josh'
2014-02-10 13:36:10,612 Trac[util] DEBUG: SQL: 
                    SELECT last_visit FROM session
                    WHERE sid=%s AND authenticated=%s
                    
2014-02-10 13:36:10,612 Trac[util] DEBUG: args: (u'josh', 1)
2014-02-10 13:36:10,612 Trac[util] DEBUG: SQL: 
                    SELECT name, value FROM session_attribute
                    WHERE sid=%s and authenticated=%s
                    
2014-02-10 13:36:10,612 Trac[util] DEBUG: args: (u'josh', 1)
2014-02-10 13:36:10,628 Trac[util] DEBUG: SQL: SELECT name FROM auth_cookie WHERE cookie=%s
2014-02-10 13:36:10,628 Trac[util] DEBUG: args: ('766967c4a0f40ab830568043280825ce',)
2014-02-10 13:36:10,628 Trac[api] DEBUG: action controllers for ticket workflow: ['ConfigurableTicketWorkflow']
2014-02-10 13:36:10,628 Trac[util] DEBUG: SQL: SELECT id, generation FROM cache
2014-02-10 13:36:10,628 Trac[util] DEBUG: SQL: SELECT generation FROM cache WHERE id=%s
2014-02-10 13:36:10,628 Trac[util] DEBUG: args: (1722364385L,)
2014-02-10 13:36:10,628 Trac[util] DEBUG: SQL: SELECT username, action FROM permission
2014-02-10 13:36:10,628 Trac[util] DEBUG: SQL: SELECT
                        id_project,name,summary,description,closed,"restrict"
                   FROM
                        smp_project
2014-02-10 13:36:10,628 Trac[util] DEBUG: SQL: SELECT
                        name
                   FROM
                        smp_project
                   WHERE
                        id_project=%s
2014-02-10 13:36:10,628 Trac[util] DEBUG: args: ['5']
2014-02-10 13:36:10,628 Trac[admin] INFO: Simple Multi Project: Modify project SST
2014-02-10 13:36:10,628 Trac[util] DEBUG: SQL: UPDATE
                        smp_project
                      SET
                        name = %s, summary = %s, description = %s, closed = %s, "restrict" = %s
                      WHERE
                        id_project = %s
2014-02-10 13:36:10,628 Trac[util] DEBUG: args: [u'SST', u'xxx', u'', 1392068154000000L, u'employees, SST-external', u'5']
2014-02-10 13:36:10,628 Trac[util] DEBUG: execute exception: DataError('integer out of range\n',)
2014-02-10 13:36:10,628 Trac[admin] ERROR: Modify Project Error: integer out of range

2014-02-10 13:36:10,628 Trac[admin] ERROR: SimpleMultiProject Error: Failed to added project 'SST'

The contents of the smp_project table:

1;"DOT";"''";"xxx";0;"employees, DOT-external"
2;"MVI3";"''";"xxx";0;"employees"
3;"QCC2";"''";"xxx";0;"employees"
4;"LAB2";"''";"xxx ";0;"employees"
5;"SST";"''";"xxx";0;"employees, SST-external"

comment:10 Changed 7 months ago by anonymous

PostgreSQL int max val = 2147483647

The question is how timestamp integer values must be saved in the right way. Any idea?

comment:11 Changed 7 months ago by endquote

Nice sleuthing. I changed smp_project.closed to a bigint and was able to close the project.

http://www.postgresql.org/docs/8.4/static/datatype-numeric.html

Last edited 7 months ago by endquote (previous) (diff)

comment:12 follow-up: Changed 7 months ago by rjollos

Trac has a database abstraction that allows a large integer to be declared in the schema using the keyword type=int64: browser:/branches/1.0-stable/trac/db_default.py@11490:114-115#L110.

It looks like the issue is that directly altering the table is not compatible across database backends: simplemultiprojectplugin/trunk/simplemultiproject/environmentSetup.py@13590:148#L146. Compare that to what is done in various Trac upgrade steps, such as browser/branches/1.0-stable/trac/upgrades/db23.py.

Last edited 7 months ago by rjollos (previous) (diff)

comment:13 Changed 7 months ago by falkb

  • Status changed from new to accepted

Thanks a lot for this excellent analysis. An update is in work now.

comment:14 Changed 7 months ago by falkb

It's like a curse, changing the column type of a table is not possible with SQLite. :( The problem is described here. It seems to me, I must solve it in the way as Rajesh suggested there, which means to copy the table to a new one.

comment:15 follow-up: Changed 7 months ago by rjollos

I'm not sure it would help even if you could change the column type for SQLite, because the database backends have different keywords for defining the datatypes.

Copying the table, so to speak, seems like the right approach, because that's how it is done in the Trac core when tables are upgraded. Make a temporary table to hold the data, drop the table, insert a new schema using the database abstraction, and then copy the data from the temporary table into the new table.

comment:16 in reply to: ↑ 15 Changed 7 months ago by falkb

Replying to rjollos:

Copying the table, so to speak, seems like the right approach,

This looks like a bigger patch. If anybody wants to help out, a patch is welcome. Likely I wont find the time for the next 2 weeks.

... insert a new schema using the database abstraction...

you mean to use just 'int64' but the rest remains the same as in the earlier upgrade, right?

comment:17 Changed 7 months ago by falkb

maybe it's just necessary to fix the plugin with another db upgrade for PostgreSQL, and maybe changing the type by altering the db table isn't a problem there...

comment:18 Changed 7 months ago by rjollos

The idea with using the classes in trac.db.schema is that they handle cross-db compatibility for you. You could probably find a way to fix this by modifying the schema for PostgreSQL, but then you may need to do the same for MySQL as well. It seems like more work, and a riskier approach, then just studying what has been done in trac:/browser/trunk/trac/upgrades and following the same approach.

comment:19 Changed 7 months ago by falkb

Hi! Please, review and test my fix. (Note: I also used the opportunity to fix the bad table column name 'restrict' as well):

  • environmentSetup.py

     
    1414 
    1515# Database schema variables 
    1616db_version_key = 'simplemultiproject_version' 
    17 db_version = 5 
     17db_version = 6 
    1818 
    1919tables = [ 
    2020    Table('smp_project', key = 'id_project') [ 
     
    157157            sqlInsertVersion = """UPDATE system SET value=%s WHERE name=%s""" 
    158158            cursor.execute(sqlInsertVersion, [db_version, db_version_key]) 
    159159            db_installed_version = 5 
     160 
     161        if db_installed_version < 6: 
     162            sql = [ 
     163            # TH ticket 11553 (problems with int range in PostgreSQL): 
     164            # fix: make field 'closed' of table 'smp_project' which is int a int64 
     165            """CREATE TEMPORARY TABLE smp_project_old AS SELECT * FROM smp_project""", 
     166            """DROP TABLE smp_project""", 
     167            """CREATE TABLE smp_project ( 
     168                     id_project      integer PRIMARY KEY, 
     169                     name            varchar(255), 
     170                     description     varchar(255), 
     171                     summary         varchar(255), 
     172                     closed          int64, 
     173                     restrict_to     text, 
     174                     UNIQUE(name) 
     175            )""" 
     176            ] 
     177 
     178            for s in sql: 
     179                cursor.execute(s) 
     180            cursor.execute("""INSERT INTO 
     181                                   smp_project 
     182                                   (id_project,name,description,summary,closed,restrict_to) 
     183                              SELECT 
     184                                   id_project,name,description,summary,closed,%s 
     185                              FROM smp_project_old""" 
     186                           % db.quote('restrict')) 
     187            cursor.execute("""DROP TABLE smp_project_old""") 
     188 
     189            sqlInsertVersion = """UPDATE system SET value=%s WHERE name=%s""" 
     190            cursor.execute(sqlInsertVersion, [db_version, db_version_key]) 
     191            db_installed_version = 6 
  • model.py

     
    6666        else: 
    6767            db = self.env.get_read_db() 
    6868        query = """SELECT 
    69                         id_project,name,summary,description,closed,%s 
     69                        id_project,name,summary,description,closed,restrict_to 
    7070                   FROM 
    7171                        smp_project 
    7272                   WHERE 
    73                         name = %%s""" % db.quote('restrict') 
     73                        name = %s""" 
    7474 
    7575        if VERSION < '0.12': 
    7676            db = self.env.get_db_cnx() 
    7777        else: 
    7878            db = self.env.get_read_db() 
    7979        cursor = db.cursor() 
    80         cursor.execute(query, [name]) 
     80        cursor.execute(query, [str(name)]) 
    8181        return  cursor.fetchone() 
    8282         
    8383    def get_all_projects(self): 
     
    8686        else: 
    8787            db = self.env.get_read_db() 
    8888        query = """SELECT 
    89                         id_project,name,summary,description,closed,%s 
     89                        id_project,name,summary,description,closed,restrict_to 
    9090                   FROM 
    91                         smp_project""" % db.quote('restrict') 
     91                        smp_project""" 
    9292 
    9393        if VERSION < '0.12': 
    9494            db = self.env.get_db_cnx() 
     
    205205        else: 
    206206            db = self.env.get_read_db() 
    207207        query    = """INSERT INTO 
    208                         smp_project (name, summary, description, closed, %s) 
    209                       VALUES (%%s, %%s, %%s, %%s, %%s);""" % db.quote('restrict') 
     208                        smp_project (name, summary, description, closed, restrict_to) 
     209                      VALUES (%s, %s, %s, %s, %s);""" 
    210210 
    211211        if VERSION < '0.12': 
    212212            db = self.env.get_db_cnx() 
     
    244244        query    = """UPDATE 
    245245                        smp_project 
    246246                      SET 
    247                         name = %%s, summary = %%s, description = %%s, closed = %%s, %s = %%s 
     247                        name = %s, summary = %s, description = %s, closed = %s, restrict_to = %s 
    248248                      WHERE 
    249                         id_project = %%s""" % db.quote('restrict') 
     249                        id_project = %s""" 
    250250 
    251251        if VERSION < '0.12': 
    252252            db = self.env.get_db_cnx() 

comment:20 follow-up: Changed 7 months ago by endquote

I installed the new version and was still able to close and open projects. I'm not sure that's really testing it, though.

comment:21 in reply to: ↑ 20 Changed 7 months ago by falkb

Replying to endquote:

I installed the new version and was still able to close and open projects. I'm not sure that's really testing it, though.

If you've applied the patch of comment:19, table smp_project should contain a column 'restrict_to' and column 'closed' should be of type in64. After all the table shouldn't have lost any entries during the conversion. And table 'system' should contain version 6 for the SMP plugin entry.

comment:22 Changed 7 months ago by endquote

I'm not having any luck applying the patched. TortoiseMerge just complains about "rejected patch hunks". If you can supply the patched file I'll give it a shot.

Changed 7 months ago by falkb

comment:23 Changed 7 months ago by falkb

endquote, would be cool if you tried with patchedFiles.zip

comment:24 Changed 7 months ago by endquote

I installed the new plugin.

C:\Users\trac\Downloads\simplemultiprojectplugin\trunk>trac-admin c:\trac\stimulant upgrade
Upgrading SimpleMultiProject database schema
SimpleMultiProject database schema version is 5, should be 6
The upgrade failed. Please fix the issue and try again.

ProgrammingError: type "int64" does not exist
LINE 6:                      closed          int64,                                         ^

comment:25 Changed 7 months ago by rjollos

The patch isn't cross-DB compatible because the DB abstraction in trac.db.api isn't being utilized. See trac:browser:/branches/1.0-stable/trac/db_default.py?rev=12533#L32 for examples.

comment:26 in reply to: ↑ 12 Changed 7 months ago by falkb

Replying to rjollos:

Trac has a database abstraction that allows a large integer to be declared in the schema using the keyword type=int64: browser:/branches/1.0-stable/trac/db_default.py@11490:114-115#L110.

I'm quite confused now because I just was doing what rjollos suggested, I used that int64.

comment:27 Changed 7 months ago by falkb

I understood int64 as the cross-platform alias for long integers. Not sure how my patch must look like to be correct.

comment:28 follow-up: Changed 7 months ago by rjollos

You need to use the Table, Column and Index classes when constructing your SQL in order to get cross-compatibility.

Last edited 7 months ago by rjollos (previous) (diff)

comment:29 in reply to: ↑ 28 Changed 7 months ago by falkb

Replying to rjollos:

You need to use the Table, Column and Index classes when constructing your SQL in order to get cross-compatibility.

???

Replying to rjollos:

The idea with using the classes in trac.db.schema is that they handle cross-db compatibility for you. You could probably find a way to fix this by modifying the schema for PostgreSQL, but then you may need to do the same for MySQL as well. It seems like more work, and a riskier approach, then just studying what has been done in trac:/browser/trunk/trac/upgrades and following the same approach.

Following this I have used trac:/browser/trunk/trac/upgrades/db5.py as template since you stated this is cross-db compatible.

comment:30 follow-ups: Changed 7 months ago by rjollos

The upgrade step db5.py is very old, from before PostgreSQL and MySQL were supported. It is not cross-db compatible.

The code in trac:browser:/branches/1.0-stable/trac/db_default.py@11490:114-115#L110 shows how to construct SQL using the Table, Column and Index classes. I can't think of a better way to show you what needs to be done other than writing the code myself, which I don't have time to do at the moment.

(I'm not sure why the text in the previous paragraph is indented. It seems to be wrapped in a blockquote).

comment:31 in reply to: ↑ 30 Changed 7 months ago by rjollos

Replying to rjollos:

The upgrade step db5.py is very old, from before PostgreSQL and MySQL were supported. It is not cross-db compatible.

Actually, what I said about PostgreSQL and MySQL not having been supported at that time may not be correct. However, if you write raw SQL, then you need to ensure it is cross-db compatible. If you use the classes I suggested, the API will handle it for you.

The problem is that you've only half-implemented what I suggested. I didn't say you could use int64 in SQL. I directed you to code showing how it can be used to construct SQL using the classes in trac.db.api.

Version 0, edited 7 months ago by rjollos (next)

comment:32 in reply to: ↑ 30 ; follow-up: Changed 7 months ago by falkb

Replying to rjollos:

The upgrade step db5.py is very old, from before PostgreSQL and MySQL were supported. It is not cross-db compatible.

I've just seen it's from 04/09/2013.

The code in trac:browser:/branches/1.0-stable/trac/db_default.py@11490:114-115#L110 shows how to construct SQL using the Table, Column and Index classes.

I just see an assignment of a table construct to a variable called schema. No context how to put that together with the quite clear way of db5.py.

I can't think of a better way to show you what needs to be done other than writing the code myself, which I don't have time to do at the moment.

I still have hope to get a sudden enlightment but it's time-consuming to find the necessary info between the lines.

comment:33 Changed 7 months ago by falkb

thanks a lot anyway for your patience and help

comment:34 in reply to: ↑ 32 Changed 7 months ago by rjollos

Replying to falkb:

I still have hope to get a sudden enlightment but it's time-consuming to find the necessary info between the lines.

Ah, the life of an open-source developer ;)

Here is a simpler example: tagsplugin/trunk/tractags/upgrades/db4.py.

comment:35 Changed 7 months ago by falkb

Looking twice at trac:/browser/branches/1.0-stable/trac/upgrades/db23.py again I can slightly imagine now this may help me as template as well... stay tuned.

comment:36 Changed 7 months ago by falkb

don't know how I get to db5.py, likely I was looking for an easier one to use as template. Anyway it's clear now that the point is to use those 3 classes on creating new tables. Thanks for that hint, Ryan.

comment:37 Changed 6 months ago by rjollos

Yeah db23.py looks like a good example.

comment:38 follow-up: Changed 6 months ago by falkb

New trial for environmentSetup.py! :-D

The file model.py is the same as from the previous patch. I wonder if I should also take the chance to add the two new desired columns 'default_cc' and 'default_milestone' of #11543 here, I would at least save one more need to upgrade.

  • simplemultiprojectplugin/trunk/simplemultiproject/environmentSetup.py

     
    1414 
    1515# Database schema variables 
    1616db_version_key = 'simplemultiproject_version' 
    17 db_version = 5 
     17db_version = 6 
    1818 
    1919tables = [ 
    2020    Table('smp_project', key = 'id_project') [ 
     
    4747    ], 
    4848] 
    4949 
     50tables_v6 = [ 
     51    Table('smp_project', key = 'id_project') [ 
     52        Column('id_project', type = 'integer', auto_increment = True), 
     53        Column('name',type = 'varchar(255)'), 
     54        Column('description',type='varchar(255)'), 
     55        Column('summary',type='varchar(255)'), 
     56        Column('closed',type='int64'), 
     57        Column('restrict_to') 
     58    ], 
     59] 
     60 
    5061""" 
    5162Extension point interface for components that need to participate in the 
    5263creation and upgrading of Trac environments, for example to create 
     
    157168            sqlInsertVersion = """UPDATE system SET value=%s WHERE name=%s""" 
    158169            cursor.execute(sqlInsertVersion, [db_version, db_version_key]) 
    159170            db_installed_version = 5 
     171 
     172        if db_installed_version < 6: 
     173            # TH ticket 11553 (problems with int range in PostgreSQL): 
     174            # fix: make field 'closed' of table 'smp_project' which is int a int64 
     175            cursor.execute("""CREATE TEMPORARY TABLE smp_project_old AS SELECT * FROM smp_project""") 
     176            cursor.execute("""DROP TABLE smp_project""") 
     177            # Create tables 
     178            for table in tables_v6: 
     179                for statement in db_connector.to_sql(table): 
     180                    cursor.execute(statement) 
     181 
     182            cursor.execute("""INSERT INTO 
     183                                   smp_project 
     184                                   (id_project,name,description,summary,closed,restrict_to) 
     185                              SELECT 
     186                                   id_project,name,description,summary,closed,%s 
     187                              FROM smp_project_old""" 
     188                           % db.quote('restrict')) 
     189            cursor.execute("""DROP TABLE smp_project_old""") 
     190 
     191            sqlInsertVersion = """UPDATE system SET value=%s WHERE name=%s""" 
     192            cursor.execute(sqlInsertVersion, [db_version, db_version_key]) 
     193            db_installed_version = 6 

P.S.: In case you already tried the patch of comment:19 you'll need to reset your trac.db to version 5 and probably rename column 'restrict_to' back to 'restrict' to get a consistent starting point.

Changed 6 months ago by falkb

second version

comment:39 in reply to: ↑ 38 ; follow-up: Changed 6 months ago by endquote

Replying to falkb:

P.S.: In case you already tried the patch of comment:19 you'll need to reset your trac.db to version 5 and probably rename column 'restrict_to' back to 'restrict' to get a consistent starting point.

How would I do that? I'm not sure where trac.db is stored.

comment:40 in reply to: ↑ 39 Changed 6 months ago by falkb

Replying to endquote:

Replying to falkb:

P.S.: In case you already tried the patch of comment:19 you'll need to reset your trac.db to version 5 and probably rename column 'restrict_to' back to 'restrict' to get a consistent starting point.

How would I do that? I'm not sure where trac.db is stored.

A trac instance consists of conf\trac.ini and in parallel is db\trac.db, you'll just need to open that trac.db with a database browser and change the version value of simplemultiprojectplugin in table 'system' from 6 back to 5. Also rename db table column 'restrict_to' back to 'restrict' in db table 'smp_project'. Then you've "reset" to the state before an upgrade from version 5 to 6.

comment:41 Changed 6 months ago by endquote

Except for my database_version is currently 29. Here is the contents of the system table:

"custom_report_manager_version";"1"
"database_version";"29"
"initial_database_version";"29"
"mastertickets";"2"
"simplemultiproject_version";"5"
"subtickets";"2"
"T&E-statuses";"assigned,accepted,closed,new,reopened"
"TimingAndEstimationPlugin_Db_Version";"6"
"tt_version";"4"
"";""

comment:42 Changed 6 months ago by falkb

OK, good. Then just check db table 'smp_project' if there's a column 'restrict' or 'restrict_to'. In case of the latter rename it to 'restrict'. Then you're ready to try the patched files model.py of attachment:patchedFiles.zip plus environmentSetup.py of attachment:patchedFiles.2.zip . Rebuild the plugin with it, reinstall that .egg file and restart Trac. Then it'll force you to upgrade which should work correctly now.

comment:43 Changed 6 months ago by endquote

The smp_project field was already "restrict".

I checked out from source, applied patches, compiled, shut down trac, installed, upgraded:

C:\Users\trac>trac-admin c:\trac\stimulant upgrade
Upgrading SimpleMultiProject database schema
SimpleMultiProject database schema version is 5, should be 6
Upgrade done.

Restarted trac and didn't see anything obvious wrong.

The smp_project field is now "restrict_to", the "closed" field is a bigint (though I had made that change myself before), and the simplemultiproject_version is 6.

Looks like everything is good?

comment:44 Changed 6 months ago by falkb

endquote, thanks a lot, you're great! Yeah, it works now. :-)

comment:45 Changed 6 months ago by falkb

  • Resolution set to fixed
  • Status changed from accepted to closed

In 13735:

  • fixed #11553: timestamps need to be bigint with PostgreSQL
  • get rid of table 'smp_project' column 'restrict' because that is a db keyword and therefore not suitable as column name, renamed it to 'restrict_to' (see #11461)
  • => plugin db schema version increased to 6

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.