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

Add Azure Container Instances infrastructure block #45

Merged
merged 106 commits into from Oct 20, 2022
Merged

Conversation

rpeden
Copy link
Contributor

@rpeden rpeden commented Sep 27, 2022

Summary

This PR adds an Azure Container Instances infrastructure block. The block provides a serverless infrastructure option on Azure similar to ECSTask on AWS and CloudRunJob on GCP.

Working

  • Flow runs on Azure Container Instances
  • Service Principal authentication
  • Adjustable memory, CPU, and GPU resources
  • Private container registries
  • Subnets
  • Output streaming

Steps to QA

  • Have an Azure account with an active subscription, or set one up. Then, on Azure:
    • Create a service principal account
    • Create a resource group
  • Run Prefect Orion
  • Add an Azure Container Credentials block document using your service principal credentials
  • Add an Azure Container Instances infrastructure block. Select the credentials you just created, then enter your resource group name and subscription ID, and set any other settings where you want to test non-default values
  • Create and apply a deployment that uses your infrastructure block.
  • Run a flow for the deployment.

QA Done

  • 100+ manual flow runs on Azure Container Instances using both Prefect Orion and Prefect Cloud
  • Created scheduled flows and ensured they run and finish reliably without errors
  • Manual testing assistance from @prefectcboyd

Relevant Issue(s)

Closes #44

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
  • This pull request includes tests or only affects documentation.
  • This pull request includes a fix, feature, enhancement, docs, collections, or maintenance label categorizing the change.

@rpeden rpeden changed the title Add aci infrastructure Add Azure Container Instances infrastructure block Sep 27, 2022
Copy link
Contributor

@ahuang11 ahuang11 left a comment

Choose a reason for hiding this comment

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

This looks really good, thanks so much for taking this on! Left a few comments and suggestions.

Also, once you start adding tests, can you also add example usage at the top of the module?
Really appreciate this!

prefect_azure/__init__.py Outdated Show resolved Hide resolved
prefect_azure/aci.py Outdated Show resolved Hide resolved
prefect_azure/aci.py Outdated Show resolved Hide resolved
prefect_azure/__init__.py Outdated Show resolved Hide resolved
prefect_azure/__init__.py Outdated Show resolved Hide resolved
prefect_azure/aci.py Outdated Show resolved Hide resolved
prefect_azure/aci.py Outdated Show resolved Hide resolved
prefect_azure/aci.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
prefect_azure/aci.py Outdated Show resolved Hide resolved
ahuang11 and others added 2 commits October 18, 2022 12:52
Co-authored-by: Michael Adkins <contact@zanie.dev>
@rpeden
Copy link
Contributor Author

rpeden commented Oct 19, 2022

Everything worked during manual testing with all the latest changes incorporated. The README examples needed minor changes but they now work as expected.

I'll also have a longer tutorial ready to publish when we merge and release this so any user that wants to try this infra block will have an end-to-end guide that walks them through it.

Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

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

This is looking great! I have a couple of requests/suggestions related to code organization and naming. If you have any questions about any of my notes feel free to reach out!

setup.py Outdated Show resolved Hide resolved
prefect_azure/credentials.py Outdated Show resolved Hide resolved
prefect_azure/credentials.py Outdated Show resolved Hide resolved
prefect_azure/container_instance.py Outdated Show resolved Hide resolved
prefect_azure/container_instance.py Outdated Show resolved Hide resolved
prefect_azure/container_instance.py Outdated Show resolved Hide resolved
prefect_azure/container_instance.py Outdated Show resolved Hide resolved
prefect_azure/container_instance.py Show resolved Hide resolved
rpeden and others added 8 commits October 19, 2022 13:18
Co-authored-by: Alexander Streed <desertaxle@users.noreply.github.com>
Co-authored-by: Alexander Streed <desertaxle@users.noreply.github.com>
Co-authored-by: Alexander Streed <desertaxle@users.noreply.github.com>
Co-authored-by: Alexander Streed <desertaxle@users.noreply.github.com>
Co-authored-by: Alexander Streed <desertaxle@users.noreply.github.com>
@ahuang11
Copy link
Contributor

Thanks for all the revisions! If a test run works with all the latest revisions, I think we can merge and make a prefect-azure release.

@rpeden
Copy link
Contributor Author

rpeden commented Oct 19, 2022

Thanks for all the revisions! If a test run works with all the latest revisions, I think we can merge and make a prefect-azure release.

No problem! It's been a pleasure working on this, and I appreciate all the helpful feedback and assistance.

@rpeden
Copy link
Contributor Author

rpeden commented Oct 20, 2022

Following up re: testing, I did another round of manual testis with the latest changes and all my container instance flows ran successfully.

@zanieb zanieb merged commit 220ec4d into main Oct 20, 2022
@rpeden rpeden deleted the add_aci_infrastructure branch December 22, 2022 17:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure Container Instance Block
4 participants