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

DITTO API using FastAPI #1

Merged
merged 23 commits into from
Jan 10, 2024
Merged

DITTO API using FastAPI #1

merged 23 commits into from
Jan 10, 2024

Conversation

tkmamidi
Copy link
Member

@tkmamidi tkmamidi commented Jan 8, 2024 •
edited by JmScherer
Loading

This repo uses FastAPI to serve DITTO as API. Please review the following:

  • Review ReadMe
  • Follow instructions in readMe to install and test API
  • Verify if DITTO score from API matches the score from DITTO-UI

Note - Most code is already reviewed in DITTO-UI repo. Only thing that are new are the main.py and query.py scripts.

@tkmamidi tkmamidi self-assigned this Jan 8, 2024
@tkmamidi tkmamidi added enhancement New feature or request good first issue Good for newcomers labels Jan 8, 2024
@wilkb777 wilkb777 self-requested a review January 9, 2024 17:48
wilkb777
wilkb777 requested changes Jan 9, 2024
View reviewed changes
Copy link
Member

@wilkb777 wilkb777 left a comment

Choose a reason for hiding this comment

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

A few minor changes to make and this should be good to go

README.md Outdated Show resolved Hide resolved
src/main.py Outdated Show resolved Hide resolved
src/utils/parse.py Outdated Show resolved Hide resolved
src/utils/query.py Outdated Show resolved Hide resolved
@JmScherer
Copy link
Contributor

GitHub actions linting appears to be complaining about the localhost url for FastAPI:

https://github.com/updownpress/markdown-lint/blob/master/rules/034-no-bare-urls.md

@JmScherer
Copy link
Contributor

FastAPI root url:

Screenshot 2024-01-09 at 1 04 50 PM

@JmScherer
Copy link
Contributor

http://localhost:8000/var/1-2406483-C-G

Screenshot 2024-01-09 at 1 21 07 PM

https://cgds-ditto.streamlit.app/

Screenshot 2024-01-09 at 1 23 39 PM

Screenshot 2024-01-09 at 1 22 31 PM

tkmamidi and others added 3 commits January 9, 2024 17:10
@wilkb777 wilkb777 self-requested a review January 10, 2024 15:16
JmScherer
JmScherer requested changes Jan 10, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
wilkb777
wilkb777 approved these changes Jan 10, 2024
View reviewed changes
Copy link
Member

@wilkb777 wilkb777 left a comment

Choose a reason for hiding this comment

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

lgtm

JmScherer
JmScherer approved these changes Jan 10, 2024
View reviewed changes
Copy link
Contributor

@JmScherer JmScherer left a comment

Choose a reason for hiding this comment

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

woo!

@tkmamidi tkmamidi merged commit f393450 into main Jan 10, 2024
3 checks passed
@tkmamidi tkmamidi deleted the api-dev branch January 10, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JmScherer JmScherer JmScherer approved these changes

@wilkb777 wilkb777 wilkb777 approved these changes

Assignees

@tkmamidi tkmamidi

Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tkmamidi @JmScherer @wilkb777