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

support the aggregate instance-type-base filter #6

Merged
merged 2 commits into from
Jun 10, 2020

Conversation

bwagner5
Copy link
Contributor

@bwagner5 bwagner5 commented Jun 9, 2020

Issue #, if available:
N/A

Description of changes:

  • Add --base-instance-type as the first aggregate filter
    • Returns similar instance-types based on the instance type passed in to construct flexible fleets if you are already using an instance type.

Example:

$ ec2-instance-selector --base-instance-type t3.large
m1.large
m3.large
m4.large
m5.large
m5a.large
m5ad.large
m5d.large
m5dn.large
m5n.large
t2.large
t3.large
t3a.large

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-commenter
Copy link

Codecov Report

Merging #6 into master will increase coverage by 0.21%.
The diff coverage is 88.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #6      +/-   ##
==========================================
+ Coverage   89.22%   89.44%   +0.21%     
==========================================
  Files           7        7              
  Lines         724      758      +34     
==========================================
+ Hits          646      678      +32     
- Misses         52       53       +1     
- Partials       26       27       +1     
Impacted Files Coverage Δ
pkg/selector/types.go 100.00% <ø> (ø)
pkg/selector/selector.go 84.07% <88.23%> (+2.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be25229...3c5f7e8. Read the comment docs.

@bwagner5 bwagner5 requested a review from haugenj June 9, 2020 22:41
Copy link
Contributor

@haugenj haugenj left a comment

Choose a reason for hiding this comment

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

Very nice ʕ•ᴥ•ʔ

@@ -66,6 +66,11 @@ const (
currentGeneration = "currentGeneration"
networkInterfaces = "networkInterfaces"
networkPerformance = "networkPerformance"

// AggregateLowPercentile is the default lower percentile for resource ranges on similar instance type comparisons
AggregateLowPercentile = 0.8
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these values derived from somewhere or did you come up with them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're arbitrary and may be adjusted in the future. I think it's a decent band for an opinionated result set though. It may make sense to adjust the low percentile to 1.0 so that the result set gives instances that are at least as spec'd out as the base.

@bwagner5 bwagner5 merged commit 101b71f into aws:master Jun 10, 2020
@bwagner5 bwagner5 deleted the instance-type-base branch July 15, 2020 23:08
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.

None yet

3 participants