Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

incorrect rse_expression length in rule_history* migrations #5846

Closed
rcarpa opened this issue Aug 29, 2022 · 1 comment · Fixed by #5892
Closed

incorrect rse_expression length in rule_history* migrations #5846

rcarpa opened this issue Aug 29, 2022 · 1 comment · Fixed by #5892
Assignees
@dchristidis
Labels
bug Database
Milestone
1.30.0

Comments

@rcarpa
Copy link
Contributor

rcarpa commented Aug 29, 2022

Motivation

The rse_expression length in "rule history" and "recent rule history" tables doesn't correspond to what is expected in models:

Ex:

rucio/lib/rucio/db/sqla/models.py

Line 1143 in 46dc0d0

rse_expression = Column(String(3000))

vs

rucio/lib/rucio/db/sqla/migrate_repo/versions/384b96aa0f60_created_rule_history_tables.py

Line 56 in 2a41a64

sa.Column('rse_expression', sa.String(255)),

This results in the table being created with wrong string length when applying migrations. Also, the length allowed in rules is different from the one allowed in rule_history.

Modification

Add another migration which correctly extends the column to have the correct length everywhere.
@bari12
Copy link
Member

bari12 commented Aug 29, 2022

I think in the oracle schema we actually defined this correctly.

rucio/etc/sql/oracle/schema.sql

Line 738 in 5454697

RSE_EXPRESSION VARCHAR2(3000),

Nevertheless this needs to be corrected for everyone else.

@dchristidis can you take care of this?

dchristidis added a commit to dchristidis/rucio that referenced this issue Sep 16, 2022
@dchristidis
Database: Correct rse_expression length rucio#5846
881dc6c 
A previous migration had the `rse_expression` column of the
`rules_hist_recent` and `rules_history` tables be created as
`String(255)`. Instead the models define them as `String(3000)`. This
commit fixes the inconsistency in the lengths.
dchristidis added a commit to dchristidis/rucio that referenced this issue Sep 23, 2022
@dchristidis
Database: Correct rse_expression length rucio#5846
3d3aac5 
A previous migration had the `rse_expression` column of the
`rules_hist_recent` and `rules_history` tables be created as
`String(255)`. Instead the models define them as `String(3000)`. This
commit fixes the inconsistency in the lengths.
@dchristidis dchristidis mentioned this issue Sep 23, 2022
Merged
@dchristidis dchristidis linked a pull request Sep 23, 2022 that will close this issue
Database: Correct rse_expression length #5846 #5892
Merged
bari12 added a commit that referenced this issue Sep 30, 2022
@bari12
Merge pull request #5892 from dchristidis/feature-5846-Database__Corr…
bfa706b 
…ect_rse_expression_length

Database: Correct `rse_expression` length #5846
@bari12 bari12 added this to the 1.30.0 "The Donkeynator" milestone Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dchristidis dchristidis

Labels
bug Database
Projects
None yet
Milestone
1.30.0
Development

Successfully merging a pull request may close this issue.

Database: Correct rse_expression length #5846 dchristidis/rucio
3 participants
@bari12 @dchristidis @rcarpa