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(examples): Added examples for Spot Event Plugin Deployment #180

Merged
merged 3 commits into from
Nov 17, 2020

Conversation

grbartel
Copy link
Contributor

Adds python and TS examples that create a farm that has the requirements to use Deadline's Spot Event Plugin.

Resolves #152


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@ddneilson ddneilson self-requested a review October 21, 2020 14:14
Copy link
Contributor

@ddneilson ddneilson 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 incomplete, unless I'm missing something.

Don't you think that we should also demonstrate creating the resources that the SEP will need? ex: A security group for the worker fleet? A LaunchConfiguration that is set up to connect to their RenderQueue, that they could then copy the json configuration from to copy into the SEP plugin settings?

# A bastion host to connect to the render farm with.
# The bastion host is for convenience (e.g. SSH into RenderQueue and WorkerFleet instances).
# This is not a critical component of the farm, so can safely be removed.
self.bastion = BastionHostLinux(
Copy link
Contributor

Choose a reason for hiding this comment

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

imo, it would be better if we strip the example down to just what's needed for the SEP. It makes the delta for adding SEP components clearer. The example can be framed as demonstrating just the SEP parts, and that the user could add the parts to the basic example.

Also reduces the amount of copy/paste that exists between the examples.

There should little to no "business-logic" constructs in this stack.
"""

def __init__(self, scope: Construct, stack_id: str, *, props: StorageTierProps, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the storage tier isn't relevant to the SEP we could either:

  1. Leave it in, but empty with a comment that directs the customer to the basic example for a demonstration of the storage tier.
  2. Remove the storage tier entirely, and leave a comment in the app that says that the storage tier was removed to focus the example and direct the customer to the basic example for the storage tier.

I.e. in either case, just use the self-contained Repository in the service tier.

@@ -0,0 +1,106 @@
# RFDK Sample Application - Deadline Spot Event Plugin

This is a sample RFDK application that deploys the basic infrastructure for a Deadline render farm. This application is structured in tiers, each representing a CloudFormation stack. The tiers are:
Copy link
Contributor

Choose a reason for hiding this comment

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

A whole bunch of this isn't accurate anymore.

# Set this value to the version of AWS Thinkbox Deadline you'd like to deploy to your farm. Deadline 10.1.9 and up are supported.
RFDK_DEADLINE_VERSION=<version_of_deadline>

npx --package=aws-rfdk@${RFDK_VERSION} stage-deadline \
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a newer form for stage-deadline that doesn't require providing the full S3 URLs. Would probably be better to use that.

!license-header.js

# The staged files for Deadline
stage
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: File should end with a newline.

There are a couple of files like this. I'll save you the copy/paste by not commenting on all of them.

@jusiskin jusiskin added the contribution/core This is a PR that came from AWS. label Oct 27, 2020
packages=setuptools.find_packages(where="package"),

install_requires=[
"aws-cdk.core==1.66.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

These are now out of date. :-(

We're on CDK 1.70.0 & RFDK 0.19.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

"typescript": "~3.9.7"
},
"dependencies": {
"@aws-cdk/core": "1.66.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Same versioning issue as in the Python example.

ddneilson
ddneilson previously approved these changes Nov 4, 2020
@grbartel
Copy link
Contributor Author

grbartel commented Nov 9, 2020

@horsmand This has all been updated

Copy link
Contributor

@horsmand horsmand left a comment

Choose a reason for hiding this comment

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

It seems you had misunderstood my last comment and removed all the additional steps for building and deploying both python and npm packages. The comment was specific to the ts example and suggested to keep the part about how to run build.sh before running the example's build.

Python needs the step you removed, where we explain how to install the locally built and packaged version of aws-rfdk if working on mainline. This is because python will always pull the package from pypi and while the release branch example will always match the package in pypi, the mainline branch will be out of sync when any unreleased features are added.
An example of these instructions are in step 3 here.

TypeScript needs the locally built aws-rfdk package, regardless of which branch is being worked off. This is because we refer to the local package from the example's tsconfig.json, always using it and never trying to fetch anything from npmjs.
An example of these instructions are in step 12 here.

@ddneilson ddneilson merged commit 49e22bf into aws:mainline Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Example Application for configuring a farm to use Deadline's SEP
4 participants