Skip to content
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

Handle smaller cr3 pdf page size #1529

Merged
merged 5 commits into from
Aug 30, 2024
Merged

Handle smaller cr3 pdf page size #1529

merged 5 commits into from
Aug 30, 2024

Conversation

johnclary
Copy link
Member

@johnclary johnclary commented Aug 27, 2024

Associated issues

Testing

URL to test: Local

Steps to test:

The problematic extract is available in the dev inbox in S3 👍

  1. Start your local stack. Use this modified version of the vision-zero helper

  2. Run the CRIS import ETL:

$ docker compose build
$ docker run -it  --rm --env-file .env --network host -v $PWD:/app cris_import-cris_import ./cris_import.py --csv --pdf --s3-download  --s3-upload
  1. You can inspect the results in your local extracts directory: ./extracts/extract_2023_[...]/crash_diagrams

Ship list

  • Check migrations for any conflicts with latest migrations in master branch
  • Confirm Hasura role permissions for necessary access
  • Code reviewed
  • Product manager approved

new_cr3_form = False
break
return new_cr3_form
if rgb_pixel[0] > 5 or rgb_pixel[1] > 5 or rgb_pixel[2] > 5:
Copy link
Member Author

@johnclary johnclary Aug 27, 2024

Choose a reason for hiding this comment

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

i was having trouble hitting pure black pixels in the smaller format, so i added this narrow tolerance of 0 to 5.

@johnclary
Copy link
Member Author

i know there's been a lot of noise here: this is ready for review again—thank you!

payload={
"records_processed": records_processed,
},
)
Copy link
Member Author

Choose a reason for hiding this comment

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

i added this so that the number of CSVs processed will be saved in the import log even if the PDF import fails. the records_processed will look something like this:

{"pdfs": 0, "units": 82, "charges": 16, "crashes": 38, "persons": 85}

and the completed_at value will be null, indicating that the import is not complete.

@chiaberry
Copy link
Member

I started following the test steps, and got an error about not finding the docker image, and I checked and my image was created this time as cris_import-cris_import 😬

@johnclary
Copy link
Member Author

johnclary commented Aug 30, 2024

ah yes—same here. i think i had both images laying around this time, but we do need to update that command if you're using docker run instead of docker compose run. i like using the docker run command because it saves my command history, rather than losing it inside the container shell

Copy link
Member

@chiaberry chiaberry left a comment

Choose a reason for hiding this comment

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

✔️

@johnclary johnclary merged commit fdb9591 into master Aug 30, 2024
9 checks passed
@chiaberry
Copy link
Member

After getting past my user errors, I got this error
requests.exceptions.InvalidSchema: No connection adapters were found for '"http://localhost:8084/v1/graphql"'

after updating my env file to remove the ", I was good to go

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