Skip to content
Snippets Groups Projects

[WIP] Modify schema, creation of new attributes

Closed Aaryan Bhagat requested to merge berserker1/mailman:modify_schema into master
1 unresolved thread

Added some new attributes changes in the Member and Mailing List model
This Merge Request is work-in-progress

  • Now I am following Mark's Implementation, see here,here and here for Mark's implementation.
Edited by Aaryan Bhagat

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
    • Author Owner

      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
    • It is number: 7 and number: 0 causing the failure. It is not something you need to fix, it is a transient bug that I have seen before.

      For now, you can just ignore this specific failure.

    • Please register or sign in to reply
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Aaryan Bhagat added 2 commits

    added 2 commits

    • d5945de3 - Fixed typos in Mailing List Model
    • 3d48455d - Modified address model and interface

    Compare with previous version

  • Aaryan Bhagat changed the description

    changed the description

  • 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
  • Aaryan Bhagat added 1 commit

    added 1 commit

    Compare with previous version

  • Aaryan Bhagat changed the description

    changed the description

  • Aaryan Bhagat changed the description

    changed the description

  • @maxking,Currently, the implementation says creating a sort of tuple (mailing list, bounce_score) pair in the Address 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

  • Aaryan Bhagat resolved all discussions

    resolved all discussions

  • Aaryan Bhagat resolved all discussions

    resolved all discussions

  • Aaryan Bhagat resolved all discussions

    resolved all discussions

  • Aaryan Bhagat resolved all discussions

    resolved all discussions

  • Aaryan Bhagat added 2 commits

    added 2 commits

    • 6c2f5642 - added necessary comment
    • da79fd3c - Modified address model and interface

    Compare with previous version

  • @maxking, @msapiro I have created a MutableList object for bounce_info attribute. Basically, it is a list object which supports Mutation Tracking.

    • See here for StackOverflow example and here for the detailed explanation in docs regarding the MutableList object.
    • Mutation Tracking as mentioned in the doc of Pickle Type.
      • To allow ORM change events to propagate for elements associated with PickleType, see Mutation Tracking.

    Edited by Aaryan Bhagat
  • If the current implementation of the Attributes is fine then

    • I will immediately create the migrations.
    • I need help in formulating tests.
  • Aaryan Bhagat resolved all discussions

    resolved all discussions

  • Mark Sapiro
  • Aaryan Bhagat added 1 commit

    added 1 commit

    Compare with previous version

  • Aaryan Bhagat resolved all discussions

    resolved all discussions

  • Mark Sapiro
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Abhilash Raj
  • Aaryan Bhagat added 1 commit

    added 1 commit

    • e49b599d - Modified Address model and interface and fixed some typos

    Compare with previous version

  • @maxking @msapiro I have made the required changes

  • Aaryan Bhagat resolved all discussions

    resolved all discussions

  • Aaryan Bhagat resolved all discussions

    resolved all discussions

  • Aaryan Bhagat added 2 commits

    added 2 commits

    Compare with previous version

  • 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 showing no support for ALTER of constraints in SQLite dialect.
    @msapiro I need some pointers in this.

  • Aaryan Bhagat added 1 commit

    added 1 commit

    Compare with previous version

  • @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_constraint

    File "/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
  • Aaryan Bhagat added 1 commit

    added 1 commit

    Compare with previous version

  • @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 in sqlite-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
  • Aaryan Bhagat added 1 commit

    added 1 commit

    Compare with previous version

  • Aaryan Bhagat mentioned in merge request !537 (closed)

    mentioned in merge request !537 (closed)

  • Aaryan Bhagat added 1 commit

    added 1 commit

    • 1566428e - Remove unique constraint migraton code, created separate pr for that

    Compare with previous version

  • Aaryan Bhagat mentioned in merge request !538 (closed)

    mentioned in merge request !538 (closed)

  • @maxking @msapiro While working on !538 (closed) I realized that bounce_info attribute in address model has some problems. Currently, one address instance has only one tuple of bounce_info but that address is subscribed to multiple MailingLists. So we are not storing individual (list_id, bounce_score) pairs. In this case, storing list_id does not matter at all and bounce_info_stale_after will not work cause the score is cumulative of all lists the address 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 Bhagat
    • Why does it have to be a single item?

      It could be something like [(tuple1), (tuple2)] and each of them could correspond to one subscription.

    • So you are implying that one address instance will have a bounce_info column entry which will be the pickled data of [(tuple1), (tuple2)....] where each tuple entry will contain the relevant details of that entry like list_id, bounce_score, last_warning.....
      This certainly is a plausible option, I can make the bounce_info as a Mutable_Dict which will have list_id: relevant_data pairs. But storing that much in one column element and we should not forget that whenever we retrieve or store this bounce_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.

    • Please register or sign in to reply
  • Aaryan Bhagat added 1 commit

    added 1 commit

    • d1b5d963 - Modified definition of bounce_info

    Compare with previous version

  • Aaryan Bhagat added 1 commit

    added 1 commit

    Compare with previous version

  • Aaryan Bhagat added 1 commit

    added 1 commit

    • 608aa0d7 - Modified address interface file

    Compare with previous version

  • Aaryan Bhagat added 1 commit

    added 1 commit

    • e0f2215d - Modified address interface file

    Compare with previous version

  • Aaryan Bhagat added 1 commit

    added 1 commit

    • 93a447aa - Modified address interface file

    Compare with previous version

  • Abhilash Raj mentioned in merge request !584 (merged)

    mentioned in merge request !584 (merged)

  • Closing this in favor of !584 (merged).

  • closed

  • Please register or sign in to reply
    Loading