Skip to content

Refactoring api #61

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.

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

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Refactoring api #61

wants to merge 3 commits into from

Conversation

paulstretenowich
Copy link
Contributor

Still need to complete ingest_delivery method

@poquirion
Copy link
Contributor

We need to make sure that nginx will let the long query/uri get to the server with somthing like this:
https://amalgjose.com/2020/05/15/how-to-set-the-allowed-url-length-for-a-nginx-request-error-code-414-uri-too-large/

Copy link
Contributor

@poquirion poquirion left a comment

Choose a reason for hiding this comment

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

Super travail @paulstretenowich!


try:
session.commit()
except SQLAlchemyError as error:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to rollback before we log errors.


try:
session.commit()
except SQLAlchemyError as error:
Copy link
Contributor

Choose a reason for hiding this comment

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

idem

raise RequestError("No 'Job' has a status, this json won't be ingested.")
try:
session.commit()
except SQLAlchemyError as error:
Copy link
Contributor

Choose a reason for hiding this comment

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

here too

session.commit()
except SQLAlchemyError as error:
logger.error("Error: %s", error)
session.rollback()
Copy link
Contributor

Choose a reason for hiding this comment

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

idem

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