Skip to content

Entry lock#53

Merged
zomglings merged 12 commits intomainfrom
entry-lock
Sep 5, 2022
Merged

Entry lock#53
zomglings merged 12 commits intomainfrom
entry-lock

Conversation

@kompotkot
Copy link
Copy Markdown
Collaborator

Changes

Design doc: https://docs.google.com/document/d/1kA57f-Xn06VMicesQ1DB56RekbAgKIQd0DcBI2UHQL0/edit#

How to test these changes?

Tested locally

Related issues

@kompotkot kompotkot requested a review from a team August 25, 2022 14:22
@kompotkot
Copy link
Copy Markdown
Collaborator Author

kompotkot commented Aug 25, 2022

@bugout-dev check

  • Run migration
  • Environment variable on AWS SSM Parameter Store: SPIRE_DB_POOL_RECYCLE_SECONDS

@kompotkot kompotkot marked this pull request as ready for review September 1, 2022 16:14
Copy link
Copy Markdown
Member

@zomglings zomglings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gg

# ### commands auto generated by Alembic - please adjust! ###
op.add_column('journal_entries', sa.Column('locked_by', sa.String(), nullable=True))
op.create_index(op.f('ix_journal_entries_locked_by'), 'journal_entries', ['locked_by'], unique=False)
op.create_unique_constraint('uq_journal_entries_id_locked_by', 'journal_entries', ['id', 'locked_by'])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id is primary key so it's already unique. This constraint doesn't add anything.

@zomglings
Copy link
Copy Markdown
Member

Migration was taking a long time because of index creation. We cancelled the migration for now. Need to update implementation so that we store a separate table with current locks.

Copy link
Copy Markdown
Member

@zomglings zomglings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gg

Copy link
Copy Markdown
Member

@zomglings zomglings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gg

Comment thread spire/journal/api.py
)


@app.delete(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be a good idea to make an explicit CREATE endpoint here, as well.

@zomglings zomglings merged commit 96876e8 into main Sep 5, 2022
@zomglings zomglings deleted the entry-lock branch September 5, 2022 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants