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

Feat/initial dev #1

Merged
merged 143 commits into from
Nov 15, 2023
Merged

Feat/initial dev #1

merged 143 commits into from
Nov 15, 2023

Conversation

RobertRosca
Copy link
Member

@RobertRosca RobertRosca commented Aug 3, 2023

The application serves as a "Zulip Write Only Proxy," providing a web API to interact with Zulip services. It allows clients to send messages, upload images, and list topics within specific streams. The application also manages client authentication and implements a JSON-based repository for data persistence.

Software Choices

Project:

  • Poetry for python project management.
  • Docker for containers.

Architecture:

  • FastAPI: as the web framework.
  • Pydantic: data validation and parsing.
  • Uvicorn as the web server.
  • Docker containers for deployment.
  • Docker compose/stacks for continuous no-downtime deployment.
    • Multiple replicas of the proxy service run at once.
    • nginx acts as load balancer in front of them.

Description

The application (roughly) follows domain-driven design principles, creating a separation between different concerns and layers. DDD isn't really used in DA so I'll write a short description of the concepts.

Domain Driven Design Summary

Domain-driven design is a software development methodology that aims to create a separation between different concerns and layers. It is based on the idea that the domain (the problem being solved) should be the main focus of the software, and that the code should be organized around it. The domain is divided into different layers, each with its own responsibilities and concerns.

The layers used in this project are:

  • Domain layer: the core business logic and domain logic of the application. It contains only the logic related to solving the problem at hand, and is independent of the other layers.
  • Application/Service layer: the layer that coordinates between the domain and data access layers. It contains the logic for creating, retrieving, and listing clients.
  • Infrastructure/Repository layer: the layer that manages data access and storage. It contains the logic for interacting with the JSON-based file storage.
  • Presentation/API layer: the layer that exposes the functionality through FastAPI endpoints. It connects the different layers and includes security measures like API key headers.

DDD is an 'onion' architecture with the domain at the core and everything else depending on the domain.

Domain Layer

The domain layer in this case is a single file models.py. It contains models for the scoped client (a Zulip client scoped to a single stream), and an admin client (an internal client which has the ability to create new scoped clients).

In DDD the domain layer is at the center of the application, and is independent of the other layers. It should not import anything from the other layers, should not be aware of infrastructure concerns like persistence/storage, and should not be aware of presentation concerns like web APIs.

Infrastructure/Repository Layer

The infrastructure layer in this case is a single file repository.py. For simplicity right now this is just a JSON-based file storage, but it could be replaced with a database or other storage solution in the future.

In DDD the infrastructure layer is responsible for data access and storage. It should be aware of domain concerns, since it returns domain objects, but should not be aware of the presentation layer.

Application/Service Layer

The application layer in this case is a single file services.py. It contains methods for creating, retrieving, and listing clients from an instance of a repository.

In DDD the application layer is responsible for coordinating between the domain and data access layers. It should be aware of both the domain and the infrastructure layers, but should not be aware of the presentation layer.

Presentation/API Layer

The presentation layer in this case is the main.py file and cli.py. It contains FastAPI endpoints and CLI commands for creating, retrieving, and listing clients. It also includes security measures like API key headers.

In DDD the presentation layer is responsible for exposing the functionality to users, in this case through a web API and through a CLI interface. It should be aware of the application/service layer, but should not be aware of the domain or infrastructure layers.

Deployment

The production deployment involves multiple replicas of the service running in front of nginx. The nginx container acts as a load balancer, distributing requests between the replicas. This allows for continuous deployment with no downtime, since the replicas can be updated one at a time.

This is all defined in a docker compose file, but compose is not used for the deployment, instead docker stacks are used. Stacks are pretty much docker compose (they use compose file) but with some fancier features available, in this case the relevant features are in the deploy section of docker-compose.yml:

    deploy:
      update_config:
        parallelism: 1
        delay: 10s
        failure_action: rollback
        order: start-first
        monitor: 5s
        max_failure_ratio: 0

This configuration means that when the stack is updated, it will start up a new instance of the container, and wait for it to be running successfully. If the update worked then the old instance is removed, if an update fail it will leave the original container running.

Since the replicas are updated one at a time, and since nginx is configured to distribute requests between them, this allows for continuous deployment with no downtime caused by the update process.

'Normal' docker compose deployments would pull in the image, stop the old container, and start the new one, which would cause downtime. If the new container fails to start then the old one is already stopped so the service would be down. Stacks allow for a more controlled update process.

Todo

Required:

  • [ ] Simplify tests: A lot of tests are redundant, test scope should be limited to the responsibilities of a layer. already wrote the tests, might as well keep them
  • Contribution documentation: poe commands, code style guide, commit style guide, formatting, type checking.
  • Fix /me endpoint for admin
  • Un-censor key for create endpoint 👀
  • Before merge, change the image tag to zulip-write-only-proxy:main (or latest? I forget which tags the metadata extractor creates)

Optional:

  • [ ] Docs? Overkill since this is so simple, but also very easy to set up. Repo is public and this kind of stream-scoped client might be useful to others using Zulip bots so maybe docs are worth including.

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@22c8a5b). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main       #1   +/-   ##
=======================================
  Coverage        ?   86.66%           
=======================================
  Files           ?        5           
  Lines           ?      195           
  Branches        ?       40           
=======================================
  Hits            ?      169           
  Misses          ?       25           
  Partials        ?        1           

@RobertRosca RobertRosca force-pushed the feat/initial-dev branch 3 times, most recently from 47f021d to 1286d8c Compare August 4, 2023 10:53
@RobertRosca RobertRosca force-pushed the feat/initial-dev branch 2 times, most recently from 45ac721 to 6abb489 Compare August 4, 2023 11:48
@RobertRosca RobertRosca marked this pull request as ready for review November 7, 2023 12:34
@RobertRosca RobertRosca requested review from matheuscteo and CammilleCC and removed request for tmichela November 7, 2023 12:35
@RobertRosca
Copy link
Member Author

@CammilleCC @matheuscteo ready for review. No major changes since you both last looked at it.

@matheuscteo
Copy link
Member

Thank you Robert, LGTM.

Copy link

@CammilleCC CammilleCC left a comment

Choose a reason for hiding this comment

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

Tested in production, thus LGTM

@RobertRosca RobertRosca merged commit 7db873a into main Nov 15, 2023
4 checks passed
@RobertRosca RobertRosca deleted the feat/initial-dev branch November 15, 2023 09:19
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