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

WIP: Rework network_route provider with resource_api #245

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

dhollinger
Copy link
Member

@dhollinger dhollinger commented May 28, 2018

do not merge - this is for review only at this time.

@igalic
Copy link
Contributor

igalic commented May 29, 2018

exciting

Copy link
Contributor

@igalic igalic left a comment

Choose a reason for hiding this comment

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

just lookin

def routes_list
routes = []
Net::IP.routes.each do |route|
routes.push(route.instance_variables.each_with_object({}) { |var, hash| hash[var.to_s.delete("@")] = route.instance_variable_get(var) })
Copy link
Contributor

Choose a reason for hiding this comment

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

it might aid readability to break this one-liner up logically into multiple lines

Copy link
Member Author

Choose a reason for hiding this comment

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

Will definitely try and do that once I have the rest of the resource working.

So far it can get the routes on a system using the net-ip ruby gem (which in turn uses ip route)

@dhollinger dhollinger force-pushed the resource_refactor branch 2 times, most recently from 32cc8e0 to 401fc2d Compare May 29, 2018 19:53
Signed-off-by: David Hollinger <[email protected]>
munge do |value|
# '255.255.255.255'.to_i will return 255, so we try to convert it back:
if value.to_i.to_s == value
# what are the chances someone is using /16 for their IPv6 network?
Copy link
Member

Choose a reason for hiding this comment

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

I do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

That file is going away ;)

That said, I am looking for a way to transform a prefix length to subnet mask. Nothing seems to do it natively (plenty of libraries to transform from subnet mask to prefix length though).

end

it 'updates the resource' do
expect(routes).to receive(:flush).with(netiproute.prefix)
Copy link
Contributor

Choose a reason for hiding this comment

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

netiproute is a double. using a property of the double to access test data seems backwards.

@vox-pupuli-tasks
Copy link

Dear @dhollinger, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@vox-pupuli-tasks
Copy link

Dear @dhollinger, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@vox-pupuli-tasks
Copy link

Dear @dhollinger, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

1 similar comment
@vox-pupuli-tasks
Copy link

Dear @dhollinger, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants