Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

ECS worker setup guide improvements #401

Merged
merged 11 commits into from
Apr 5, 2024
Merged

Conversation

jeanluciano
Copy link
Contributor

@jeanluciano jeanluciano commented Mar 26, 2024

Closes #394

Example

Graph updated to used Prefect colors, open to different color values:

%%{
  init: {
    'theme': 'base',
    'themeVariables': {
      'primaryColor': '#2D6DF6',
      'primaryTextColor': '#fff',
      'lineColor': '#FE5A14',
      'secondaryColor': '#E04BF0',
      'tertiaryColor': '#fff'
    }
  }
}%%
graph TB

  subgraph ecs_cluster[ECS cluster]
    subgraph ecs_service[ECS service]
      td_worker[Worker task definition] --> |defines| prefect_worker((Prefect worker))
    end
    prefect_worker -->|kicks off| ecs_task
    fr_task_definition[Flow run task definition]


    subgraph ecs_task["ECS task execution"]
    style ecs_task text-align:center,display:flex
    
   
    flow_run((Flow run))

    end
    fr_task_definition -->|defines| ecs_task
  end

  subgraph prefect_cloud[Prefect Cloud]
    subgraph prefect_workpool[ECS work pool]
      workqueue[Work queue]
    end
  end

  subgraph github["ECR"]
    flow_code{{"Flow code"}}
  end
  flow_code --> |pulls| ecs_task
  prefect_worker -->|polls| workqueue
  prefect_workpool -->|configures| fr_task_definition
Loading

Screenshots

Checklist

  • [ x ] References any related issue by including "Closes #" or "Closes ".
    • If no issue exists and your change is not a small fix, please create an issue first.
  • Includes tests or only affects documentation.
  • Passes pre-commit checks.
    • Run pre-commit install && pre-commit run --all locally for formatting and linting.
  • Includes screenshots of documentation updates.
    • Run mkdocs serve view documentation locally.

Copy link
Contributor

@discdiver discdiver left a comment

Choose a reason for hiding this comment

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

Nice! I like the direction!

  • Could you please add a screenshot of the Mermaid diagram to the PR?
  • Looks like there are some merge conflicts that need resolved.

docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
@jeanluciano jeanluciano marked this pull request as ready for review March 28, 2024 18:45
@jeanluciano jeanluciano requested a review from a team as a code owner March 28, 2024 18:45
@kevingrismore
Copy link
Contributor

There's a reference on line 5 to "Prefect 2" which I think we can change to just "Prefect"

docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
Copy link
Contributor

@kevingrismore kevingrismore left a comment

Choose a reason for hiding this comment

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

Mostly minor style things.

Thank you so much @jeanluciano for taking this on!!!

docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Show resolved Hide resolved
docs/ecs_guide.md Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
Copy link
Contributor

@discdiver discdiver left a comment

Choose a reason for hiding this comment

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

Looking good. A few comments/suggestions.

docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
docs/ecs_guide.md Outdated Show resolved Hide resolved
Copy link
Contributor

@discdiver discdiver left a comment

Choose a reason for hiding this comment

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

Thank you! Added one suggested change for title case consistency.


### 5. Find the deployment in the UI and click the **Quick Run** button!

## Optional Next Steps
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Optional Next Steps
## Optional next steps

@jeanluciano jeanluciano merged commit 7ea9679 into main Apr 5, 2024
7 checks passed
@jeanluciano jeanluciano deleted the ecs-worker-setup-guide-fixes branch April 5, 2024 16:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improvement to ECS worker setup guide
4 participants