Skip to content

Conversation

@HrikB
Copy link
Contributor

@HrikB HrikB commented Apr 21, 2023

No description provided.

@HrikB HrikB requested a review from shahthepro April 21, 2023 04:28

def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
# op.drop_table('bitcoin_prices')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove these commented out lines from the migration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.create_index('ix_token_info_created_at', 'token_info', ['created_at'], unique=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

If these are commented out during the upgrade, let's not "undo" them during downgrade

@HrikB HrikB requested a review from shahthepro April 21, 2023 04:34
@HrikB HrikB requested a review from franckc April 21, 2023 06:08

@cross_origin()
@app.route("/oeth-subscribe", methods=["POST"], strict_slashes=False)
def oeth_subscribe():
Copy link
Contributor

@shahthepro shahthepro Apr 21, 2023

Choose a reason for hiding this comment

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

nit: Since only source differs between this function and the other one, why one extract this into a common function to reduce code duplication?

@micahalcorn micahalcorn removed the request for review from franckc April 21, 2023 17:40
@rafaelugolini
Copy link
Contributor

Do we need to upgrade the libraries from Snyk? @HrikB @shahthepro

@shahthepro
Copy link
Contributor

Do we need to upgrade the libraries from Snyk? @HrikB @shahthepro

We could do that, we also have dependabot on this repo that has some PR with the upgrades. However, we need to test it out to make sure it doesn't break anything else. Would be better off doing it in a different PR

@HrikB HrikB merged commit d533061 into master Apr 21, 2023
@HrikB HrikB deleted the oeth-subscribe branch April 21, 2023 22:37
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.

4 participants