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

refactor: Replace Bottle with FastAPI #668

Merged
merged 14 commits into from
Apr 26, 2024
Merged

refactor: Replace Bottle with FastAPI #668

merged 14 commits into from
Apr 26, 2024

Conversation

taddes
Copy link
Contributor

@taddes taddes commented Mar 23, 2024 •
edited
Loading

Closes SYNC-4188

Turns out Docker interface didn't like the IP of localhost, yet 127.0.0.1 worked. Maybe an issue in IP resolution since Ipv6 interfaces checked first?

Note: Will need to rebase async changes in #667 over this before merge.

@taddes taddes self-assigned this Mar 23, 2024
@taddes taddes marked this pull request as draft March 23, 2024 01:07
@taddes
Copy link
Contributor Author

taddes commented Mar 25, 2024

Ugh, clearly an error in the Docker image that does not occur locally, investigating.

@taddes taddes force-pushed the refactor/SYNC-4188_replace_bottle_with_fastapi branch 3 times, most recently from a77030b to 9969521 Compare April 19, 2024 19:13
@taddes taddes marked this pull request as ready for review April 19, 2024 19:29
@taddes taddes requested review from pjenvey and jrconlin April 19, 2024 19:30
@jrconlin
Copy link
Member

Some docker images do not have any dnsutils installed, or the host /etc/hosts file doesn't declare localhost so they have no way to identify localhost. Using an IP address helps. I suppose we could probably try a few 'clever' work around, but it's probably not worth it.

@taddes taddes force-pushed the refactor/SYNC-4188_replace_bottle_with_fastapi branch from 9969521 to afe30cb Compare April 19, 2024 22:59
@taddes taddes force-pushed the refactor/SYNC-4188_replace_bottle_with_fastapi branch from b2ea6c6 to 3180e44 Compare April 25, 2024 20:27
@taddes taddes mentioned this pull request Apr 25, 2024
Closed
@taddes taddes requested a review from jrconlin April 25, 2024 20:46
jrconlin
jrconlin reviewed Apr 25, 2024
View reviewed changes
tests/integration/dual_test.json
@@ -7,4 +7,4 @@
"db_settings": "{\"message_table\": \"message_int_test\",\"router_table\": \"router_int_test\"}",
"dsn": "http://localhost:8000/"
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Was removing this last \n intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My auto format for some reason did so! Probably an IDE "feature". Is there a preference one way or another? Happy to change it if we prefer a trailing line.

Copy link
Member

Choose a reason for hiding this comment

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

no worries. It's a JSON file so not super critical. Just curious if there was some different reason.

@taddes taddes changed the title Replace Bottle with FastAPI refactor: Replace Bottle with FastAPI Apr 26, 2024
@taddes taddes merged commit d49458e into master Apr 26, 2024
1 check passed
@taddes taddes deleted the refactor/SYNC-4188_replace_bottle_with_fastapi branch April 26, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrconlin jrconlin jrconlin approved these changes

@pjenvey pjenvey Awaiting requested review from pjenvey

Assignees

@taddes taddes

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@taddes @jrconlin