Skip to content

Conversation

@qued
Copy link
Contributor

@qued qued commented Jan 3, 2023

Small factorization, primarily moving some of the api processing code out to where it can be reused.

Testing:

Functionality should be unchanged from a user perspective.

Tests should pass.

Curl should work while app is running locally:

  • Run make run-app-dev
  • Run the following curls from the pipeline-oer/sample-docs directory, or replace fake-oer.pdf with the path to another pdf:
curl -X 'POST' \
  'localhost:8000/layout/pdf' \
  -H 'accept: application/json' \
  -H 'Content-Type: multipart/form-data' \
  -F 'file=@fake-oer.pdf' | jq -C . | less -R

Should succeed and return narrative sections.

curl -X 'POST' \
  'localhost:8000/layout/pdf' \
  -H 'accept: application/json' \
  -H 'Content-Type: multipart/form-data' \
  -F 'model=checkbox' \
  -F 'file=@fake-oer.pdf' | jq -C . | less -R

Should succeed and return checkboxes (should be present if fake-oer.pdf is used).

curl -X 'POST' \
  'localhost:8000/layout/pdf' \
  -H 'accept: application/json' \
  -H 'Content-Type: multipart/form-data' \
  -F 'model=fake' \
  -F 'file=@fake-oer.pdf' | jq -C . | less -R

Should fail with "unknown model" error message.

@qued qued requested a review from MthwRobinson January 3, 2023 17:02
Copy link
Contributor

@MthwRobinson MthwRobinson left a comment

Choose a reason for hiding this comment

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

LGTM. I like UnknownModelException addition.

@@ -1,12 +1,14 @@
import pytest
from unittest.mock import patch
from unittest.mock import patch, mock_open
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't know about mock_open before!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Me either!

return model_path, config_path, label_map


class UnknownModelException(Exception):
Copy link
Contributor

Choose a reason for hiding this comment

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

:-)

@qued qued merged commit 605c1f2 into main Jan 3, 2023
@qued qued deleted the alan/refactor-for-local branch January 3, 2023 19:38
benjats07 pushed a commit that referenced this pull request Jan 10, 2023
Bumps [ipython](https://github.com/ipython/ipython) from 8.5.0 to 8.6.0.
- [Release notes](https://github.com/ipython/ipython/releases)
- [Commits](ipython/ipython@8.5.0...8.6.0)

---
updated-dependencies:
- dependency-name: ipython
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

3 participants