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

docs(edge): Add @clayms examples #495

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gabriel-tessier
Copy link
Collaborator

I added @clayms examples as he is certainly busy and didn't push any PR so far, except if I mess it.

@mingrammer
I mainly copy paste what @clayms wrote in the issues, maybe it's better that he review as he authored the code, I tested and generated the png files so the code is correct but maybe there's some missing comments.
Original discussion started here:
#433

Also in the bellow issue he listed a lot of examples but I'm not sure that it will help to add it in the doc as it's more related to graphviz attributes that Diagrams only use. Sorry if I'm not clear but what I want to say is if graphviz make some breaking changes with this attributes it will broke Diagrams and Diagrams can't do anything about.
WDYT?

@clayms
Copy link

clayms commented Mar 29, 2021

Thanks @gabriel-tessier

Knowing what I now know, for the first example I would change the Custom class node to simply a Node class node so the end user would not have to create their own "transparent.png" file. I would then set the Node width and height to "0" so the Edges carried straight through uninterrupted.

Then only change the text to reflect that.

Also replace the word "slave."

I have made some other minor edits to the text inline.

Full code below.

from diagrams import Cluster, Diagram, Edge, Node
from diagrams.onprem.analytics import Spark
from diagrams.onprem.compute import Server
from diagrams.onprem.database import PostgreSQL
from diagrams.onprem.inmemory import Redis
from diagrams.onprem.aggregator import Fluentd
from diagrams.onprem.monitoring import Grafana, Prometheus
from diagrams.onprem.network import Nginx
from diagrams.onprem.queue import Kafka


with Diagram("\nAdvanced Web Service with On-Premise Less edges", show=False):
    ingress = Nginx("ingress")

    with Cluster("Service Cluster"):
            serv1 = Server("grpc1")
            serv2 = Server("grpc2")
            serv3 = Server("grpc3")

    with Cluster(""):
        blankHA = Node("", shape="plaintext", width="0", height="0")
        
        metrics = Prometheus("metric")
        metrics << Grafana("monitoring")
        
        aggregator = Fluentd("logging")
        blankHA >> aggregator >> Kafka("stream") >> Spark("analytics")
        
        with Cluster("Database HA"):
            master = PostgreSQL("users")
            master - PostgreSQL("replica") << metrics
            blankHA >> master
        
        with Cluster("Sessions HA"):
            master = Redis("session")
            master - Redis("replica") << metrics
            blankHA >> master

    ingress >> serv2 >> blankHA

image

Copy link

@clayms clayms left a comment

Choose a reason for hiding this comment

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

bulk of code is in a comment to the whole PR.

docs/guides/edge.md Outdated Show resolved Hide resolved
docs/guides/edge.md Outdated Show resolved Hide resolved
docs/guides/edge.md Outdated Show resolved Hide resolved
docs/guides/edge.md Outdated Show resolved Hide resolved
docs/guides/edge.md Outdated Show resolved Hide resolved
@gabriel-tessier
Copy link
Collaborator Author

gabriel-tessier commented Mar 29, 2021

I think I didn't mess up too much and applied all your requested changes without mistakes.
I thought I can apply the comment you made in the review but somehow I couldn't. So I copy paste and commit again, I only hope that I forget nothing.
I also re-generate the 2 diagrams png files.

Thanks again for the review.

@mingrammer mingrammer added kind/docs Improvements or additions to documentation status/need-to-review Need to review labels Nov 8, 2022
@tvqphuoc01
Copy link
Collaborator

@gabriel-tessier seem like we have some conflict here.

@gabriel-tessier
Copy link
Collaborator Author

@tvqphuoc01

conflict solved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/docs Improvements or additions to documentation status/need-to-review Need to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants