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

Fix sample Docker Compose environment #455

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Fix sample Docker Compose environment #455

merged 1 commit into from
Sep 27, 2024

Conversation

ceruleandeep
Copy link

@ceruleandeep ceruleandeep commented Sep 27, 2024 •
edited by tazlin
Loading

Relates to #448

  • Pull Horde's custom postgres image
  • Build aihorde Docker image in two stages to minimize rebuild time
  • Establish minimal necessary configuration to bring up a working composition
  • Fix pre-commit config so black and ruff hooks run
  • Catch and report exceptions in is_redis_up()
  • Add missing final EOL markers to some files (per black)
  • Update documentation

@tazlin tazlin added the allow-ci A PR with this label will run through CI. label Sep 27, 2024
@tazlin
Copy link
Member

tazlin commented Sep 27, 2024 •
edited
Loading

Note that the runner is failing due to the issue identified and fixed in #454. As such, this PR should wait to be merged until #454 is on main, at which point this PR should be rebased and the testing re-run. Done.

Note also that merging this PR would supersede #447.

@tazlin
Fix example Docker Compose environment
9ce5b55 
Pull Horde's custom postgres image

Build aihorde Docker image in two stages to minimise rebuild time

Establish minimal necessary configuration to bring up a working composition

Fix pre-commit config so black and ruff hooks run

Catch and report exceptions in is_redis_up()

Add missing final EOL markers to some files (per black)

Update documentation
tazlin
tazlin approved these changes Sep 27, 2024
View reviewed changes
Copy link
Member

@tazlin tazlin left a comment

Choose a reason for hiding this comment

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

The substantive docker changes all work locally for me and represent an improvement over the prior dockerfile/compose definitions particularly in clarity. Notably, the compose did not work out of the box, but now does (and with better practices than before).

The code change to redis_ctrl.py should only affect a very narrow set of circumstances and shouldn't have side effects in production today.

@tazlin tazlin requested a review from db0 September 27, 2024 12:58
db0
db0 approved these changes Sep 27, 2024
View reviewed changes
Copy link
Member

@db0 db0 left a comment

Choose a reason for hiding this comment

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

LGTM!

@db0 db0 merged commit aaba202 into Haidra-Org:main Sep 27, 2024
4 checks passed
@tazlin tazlin mentioned this pull request Sep 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@db0 db0 db0 approved these changes

@tazlin tazlin tazlin approved these changes

Assignees
No one assigned
Labels
allow-ci A PR with this label will run through CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ceruleandeep @tazlin @db0