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.

Sign up for GitHub

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

Jump to bottom

OddpubMetrics Docker image and Database Table Generation #15

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

OddpubMetrics Docker image and Database Table Generation #15

wants to merge 24 commits into from

Conversation

quang-ng
Copy link
Collaborator

Working on issue #14

@quang-ng
Add oddpub_metrics table and model definition
1182a9c
@quang-ng
create Oddpub API
580d6c4
@quang-ng
Enhance Dockerfile and pyproject.toml for R support and author attrib…
ac53e92 
…ution
@quang-ng
Update pyproject.toml to include pandas and numpy dependencies
157a217
@quang-ng
Refactor Dockerfile and update pyproject.toml to enhance R package in…
05180ed 
…stallation
@quang-ng
Add FastAPI application and Docker setup for ODDPub processing
f20241a
@quang-ng
Enhance logging and update PDF processing in Oddpub application
d7e2115
@quang-ng
Add OddpubWrapper class for PDF processing and API integration
1bb19ec
@quang-ng
Update Oddpub metrics to allow nullable fields and enhance OddpubWrap…
ae524a7 
…per for PDF processing
@quang-ng
Refactor R result conversion in OddpubWrapper to use pandas2ri for im…
789cc2d 
…proved data handling
@quang-ng
Add unit tests for OddpubWrapper and refactor test setup in RTranspar…
9943c5f 
…entDataUploader
@quang-ng quang-ng marked this pull request as ready for review December 13, 2024 10:32
@joshlawrimore
Copy link
Contributor

@leej3 can you test if the docker image installs oddpub in R in on your machine or @quang-ng can you write a GitHub action that tests it? I cannot build the latest image without the install of oddpub failing on my machine. Thanks!

@quang-ng
Implement a new test for OddpubWrapper to validate PDF processing wit…
f20dbdb 
…hout mock API, ensuring correct data retrieval from the database.
@quang-ng
Copy link
Collaborator Author

@leej3 can you test if the docker image installs oddpub in R in on your machine or @quang-ng can you write a GitHub action that tests it? I cannot build the latest image without the install of oddpub failing on my machine. Thanks!

done, please help me review it

leej3
leej3 requested changes Dec 18, 2024
View reviewed changes
Copy link
Contributor

@leej3 leej3 left a comment

Choose a reason for hiding this comment

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

Thanks @quang-ng, it looks pretty solid! Some details to address but it's almost ready.

.docker/postgres-compose.yaml Outdated Show resolved Hide resolved
services/oddpub/README.md Outdated Show resolved Hide resolved
services/oddpub/README.md Outdated Show resolved Hide resolved
services/oddpub/README.md Outdated Show resolved Hide resolved
services/oddpub/_entrypoint.sh Show resolved Hide resolved
.docker/postgres-compose.yaml Outdated Show resolved Hide resolved
services/oddpub/README.md Show resolved Hide resolved
.mockenv Outdated Show resolved Hide resolved
dsst_etl/oddpub_wrapper.py Outdated Show resolved Hide resolved
tests/test_oddpub.py Outdated Show resolved Hide resolved
@quang-ng quang-ng requested a review from leej3 December 23, 2024 08:58
leej3
leej3 approved these changes Dec 23, 2024
View reviewed changes
Copy link
Contributor

@leej3 leej3 left a comment

Choose a reason for hiding this comment

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

@joshlawrimore could you merge this if you are happy with it.

LGTM. Thanks @quang-ng!

services/oddpub/README.md Outdated Show resolved Hide resolved
@quang-ng
Copy link
Collaborator Author

Thank you @leej3 👍

@quang-ng quang-ng mentioned this pull request Dec 24, 2024
Open
@quang-ng quang-ng self-assigned this Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leej3 leej3 leej3 approved these changes

@joshlawrimore joshlawrimore Awaiting requested review from joshlawrimore

Assignees

@quang-ng quang-ng

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quang-ng @joshlawrimore @leej3