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.

Sign up for GitHub

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

fog-core 2.x, fog-json 1.x #433

Merged
merged 4 commits into from
Apr 16, 2018
Merged

fog-core 2.x, fog-json 1.x #433

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ec620d8
fog-core 2.x, fog-json 1.x
lanej Feb 26, 2018
199cbf8
fix(elb): override #all_associations_and_attributes
lanej Mar 14, 2018
770629e
style(elb): reduce branch conditions and increase readability
lanej Mar 14, 2018
24ea467
fix: remove Volume#server= to attach volumes
lanej Apr 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/fog/aws/models/elb/backend_server_descriptions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def all
end

def get(instance_port)
all.find{|e| e.instance_port == instance_port}
all.find { |e| e.instance_port == instance_port }
end
end
end
Expand Down
7 changes: 3 additions & 4 deletions lib/fog/aws/models/elb/listeners.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,16 @@ def all
end

def get(lb_port)
all.find{|listener| listener.lb_port == lb_port}
all.find { |listener| listener.lb_port == lb_port }
end

private

# Munge an array of ListenerDescription hashes like:
# {'Listener' => listener, 'PolicyNames' => []}
# to an array of listeners with a PolicyNames key
def munged_data
data.map {|description|
description['Listener'].merge('PolicyNames' => description['PolicyNames'])
}
data.map { |description| description['Listener'].merge('PolicyNames' => description['PolicyNames']) }
end
end
end
Expand Down
12 changes: 10 additions & 2 deletions lib/fog/aws/models/elb/load_balancer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,16 +223,24 @@ def save
end

def reload
super
@instance_health = nil
@policy_descriptions = nil
self
super
end

def destroy
requires :id
service.delete_load_balancer(id)
end

protected

def all_associations_and_attributes
super.merge(
'ListenerDescriptions' => attributes['ListenerDescriptions'],
'BackendServerDescriptions' => attributes['BackendServerDescriptions'],
)
end
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need this? Maybe we need to improve this method on fog-core?

Copy link
Member Author

@lanej lanej Mar 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Attributes are not registered by an attribute declaration but are used to generate to collections from the underlying attributes data.

This seems like a reasonable workaround using normal OO methodology.

Copy link
Member

Choose a reason for hiding this comment

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

Got it. I'm not really pointing this as a problem but i'm more interested in knowing the reasons behind it. If it does inflect an in improvement in fog-core i would be more then happy to do it myself. =)

Are those attributes not being used by the DSL for a particular reason? Is there something i can improve in fog-core to help improve the usage?

Copy link
Member Author

Choose a reason for hiding this comment

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

They seem to describe related resources but not this resource, which is my they are not included as attributes. If other providers are having this issue, we could introduce a "hidden attribute" (data we want to keep but does not describe the current model).

Copy link
Member

Choose a reason for hiding this comment

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

Hmmm. So you mean an attribute that ideally does not belong to this resource but the API expects? Like a concept issue from the API standpoint?

BTW, this is not a blocker. I'm just trying to figure out the use case to see if/how we can improve fog-core. My main goal now is to get it to a better state.

end
end
end
Expand Down
11 changes: 5 additions & 6 deletions lib/fog/aws/models/elb/load_balancers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ class LoadBalancers < Fog::Collection
model Fog::AWS::ELB::LoadBalancer

# Creates a new load balancer
def initialize(attributes={})
def initialize(attributes = {})
super
end

def all
result = []
marker = nil
finished = false
while !finished
until finished
data = service.describe_load_balancers('Marker' => marker).body
result.concat(data['DescribeLoadBalancersResult']['LoadBalancerDescriptions'])
marker = data['DescribeLoadBalancersResult']['NextMarker']
Expand All @@ -24,10 +24,9 @@ def all
end

def get(identity)
if identity
data = service.describe_load_balancers('LoadBalancerNames' => identity).body['DescribeLoadBalancersResult']['LoadBalancerDescriptions'].first
new(data)
end
return unless identity
data = service.describe_load_balancers('LoadBalancerNames' => identity).body['DescribeLoadBalancersResult']['LoadBalancerDescriptions'].first
new(data)
rescue Fog::AWS::ELB::NotFound
nil
end
Expand Down
45 changes: 24 additions & 21 deletions lib/fog/aws/requests/elb/create_load_balancer_listeners.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,34 +51,37 @@ def create_load_balancer_listeners(lb_name, listeners)

class Mock
def create_load_balancer_listeners(lb_name, listeners)
if load_balancer = self.data[:load_balancers][lb_name]
response = Excon::Response.new
load_balancer = data[:load_balancers][lb_name]
raise Fog::AWS::ELB::NotFound unless load_balancer
response = Excon::Response.new

certificate_ids = Fog::AWS::IAM::Mock.data[@aws_access_key_id][:server_certificates].map {|n, c| c['Arn'] }
certificate_ids = Fog::AWS::IAM::Mock.data[@aws_access_key_id][:server_certificates].map { |_n, c| c['Arn'] }

listeners.each do |listener|
if listener['SSLCertificateId'] and !certificate_ids.include? listener['SSLCertificateId']
raise Fog::AWS::IAM::NotFound.new('CertificateNotFound')
end
listeners.each do |listener|
if listener['SSLCertificateId'] && !certificate_ids.include?(listener['SSLCertificateId'])
raise Fog::AWS::IAM::NotFound, 'CertificateNotFound'
end

if (%w( HTTP HTTPS).include?(listener['Protocol']) && !%w( HTTP HTTPS ).include?(listener['InstanceProtocol'])) ||
(%w( TCP SSL).include?(listener['Protocol']) && !%w( TCP SSL ).include?(listener['InstanceProtocol']))
raise Fog::AWS::ELB::ValidationError
end if listener['Protocol'] && listener['InstanceProtocol']
load_balancer['ListenerDescriptions'] << {'Listener' => listener, 'PolicyNames' => []}
if listener['Protocol'] && listener['InstanceProtocol']
if (
%w[HTTP HTTPS].include?(listener['Protocol']) && !%w[HTTP HTTPS].include?(listener['InstanceProtocol'])
) || (
%w[TCP SSL].include?(listener['Protocol']) && !%w[TCP SSL].include?(listener['InstanceProtocol'])
)
raise Fog::AWS::ELB::ValidationError
end
end
load_balancer['ListenerDescriptions'] << { 'Listener' => listener, 'PolicyNames' => [] }
end

response.status = 200
response.body = {
'ResponseMetadata' => {
'RequestId' => Fog::AWS::Mock.request_id
}
response.status = 200
response.body = {
'ResponseMetadata' => {
'RequestId' => Fog::AWS::Mock.request_id
}
}

response
else
raise Fog::AWS::ELB::NotFound
end
response
end
end
end
Expand Down