[WIP] Modify schema, creation of new attributes
Merge request reports
Activity
I do not know what
bounce_matching_headers
variable is, nothing of that sort is mentioned in the docs.Hence no attribute is written for it. Also aside from writing the attribute description, what else should be done to complete the process?
Pointers on this would be helpful. I required to post this in the mailing list, please say so and I will do that effectively immediately.The descriptions I have taken from this page of the org, some definitions I have modified according to the proposal.
Also, the pysql pipeline is failing and the error is :
Differences (ndiff with -expected +actual):
- http_etag: "..."
+ http_etag: "0f88179671a971224f1cb6709652523693326f65"
- number: 0
? ^
+ number: 7
? ^
Shouldn't
"..."
ignore the output?Edited by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Abhilash Raj
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
added 2 commits
I have not created the migrations using Alembic reason being I want to confirm that I want to rectify any mistakes as of now and create the migrations else I would I have to create them again and again.
Edited by Aaryan Bhagat@maxking,Currently, the implementation says creating a sort of tuple
(mailing list, bounce_score)
pair in theAddress
Model (see here).I think creating
2 Columns
for this would be a solution. We will also need to modify some functions as when the email is subscribed to a mailing list the 2 column should be updated (I am not sure which functions though).
I am not sure right now.
I need some pointers on the implementation- Resolved by Aaryan Bhagat
I think only one additional column in the address model is necessary. This should be a possibly empty list of items. There are a couple of possibilities for the items. In Mailman 2.1 they are instances of a _BounceInfo class. See https://bazaar.launchpad.net/~mailman-coders/mailman/2.1/view/head:/Mailman/Bouncer.py#L56. That information would need to be augmented with the list id. The info could also just be a tuple rather than a class instance.
added 2 commits
@maxking, @msapiro I have created a
MutableList
object forbounce_info
attribute. Basically, it is a list object which supports Mutation Tracking.Edited by Aaryan Bhagat- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
- Resolved by Aaryan Bhagat
added 1 commit
- e49b599d - Modified Address model and interface and fixed some typos
- Resolved by Aaryan Bhagat
This lgtm, can you add a migration to this and keep moving with your implementation?
added 2 commits
I have created the migrations but the SQL tests are failing, I am not so sure about the reason, for example, the test for
sqlite-37
is showingno support for ALTER of constraints in SQLite dialect
.
@msapiro I need some pointers in this.@berserker1 See example migrations in Core. Also, you can search more about the exception and you will find a lot of results.
Basic TL;DR is that SQLite database doesn't support Alter Constraints and many similar SQL statements. You need to if-else based on the dailect of database being used. You can find examples in Core.
So the main reason for the error is
File "/builds/berserker1/mailman/src/mailman/database/alembic/versions/58858f75303c_added_new_bounce_attributes.py", line 30, in downgrade
op.drop_constraint(None, 'domain', type_='unique')
File "", line 8, in drop_constraint
File "", line 3, in drop_constraintFile "/builds/berserker1/mailman/.tox/py37-nocov/lib/python3.7/site-packages/alembic/operations/ops.py", line 156, in drop_constraint
return operations.invoke(op)File "/builds/berserker1/mailman/.tox/py37-nocov/lib/python3.7/site-packages/alembic/operations/base.py", line 345, in invoke
return fn(self, operation)File "/builds/berserker1/mailman/.tox/py37-nocov/lib/python3.7/site-packages/alembic/operations/toimpl.py", line 161, in drop_constraint
schema=operation.schema,File "/builds/berserker1/mailman/.tox/py37-nocov/lib/python3.7/site-packages/alembic/ddl/sqlite.py", line 46, in drop_constraint
"No support for ALTER of constraints in SQLite dialect"What I do not get is why Alembic is creating the constraints of
Domains
model when I did not modify the code for it all?Should I just remove the migrations of the domain model?
Edited by Aaryan Bhagat@msapiro @stephenjturnbull I made some improvements regarding the SQLite error, mostly by doing
batch_alter_table
as the way around suggested by SQLite itself. Currently, all the previous errors were gone now and a new single error popped up and I am having difficulties with this one. Pointers on this error would be helpful.
I am specifically talking about the error insqlite-37
job of the pipeline.Edited by Aaryan Bhagat@maxking @msapiro Currently, I seem to have trouble grasping the nature of this error.
- The test case is failing, it says that the column
address.bounce_info
does not exist but in the code alembic already downgrades it. - I have used the
batch_alter_table
for making changes. Where the implementation is going wrong, I do not know as of now? - Help in this would be appreciated, it has taken a toll in the speed of completion of the project.
Edited by Aaryan Bhagat- The test case is failing, it says that the column
mentioned in merge request !537 (closed)
added 1 commit
- 1566428e - Remove unique constraint migraton code, created separate pr for that
mentioned in merge request !538 (closed)
@maxking @msapiro While working on !538 (closed) I realized that
bounce_info
attribute inaddress
model has some problems. Currently, oneaddress
instance has only one tuple ofbounce_info
but that address is subscribed to multipleMailingLists
. So we are not storing individual(list_id, bounce_score)
pairs. In this case, storinglist_id
does not matter at all andbounce_info_stale_after
will not work cause the score is cumulative of all lists theaddress
is subscribed.I think another table for
bounce_info
would be required.
Is the above justification correct?
Am I missing something here?Edited by Aaryan BhagatSo you are implying that one
address
instance will have abounce_info
column entry which will be thepickled
data of[(tuple1), (tuple2)....]
where eachtuple
entry will contain the relevant details of that entry likelist_id, bounce_score, last_warning....
.
This certainly is a plausible option, I can make thebounce_info
as aMutable_Dict
which will havelist_id: relevant_data
pairs. But storing that much in one column element and we should not forget that whenever we retrieve or store thisbounce_info
pickle will always work to load or dump it. I do not know how it will affect performance.
But with no other viable options, I am giving this a go.Upon reading the docs of sqlalchemy, there are certain problems with
MutableDict
class, it cannot change the data sometimes. You can see this Google group link for reference. I will stick to the list implementation only.
mentioned in merge request !584 (merged)
Closing this in favor of !584 (merged).