Skip to content

Conversation

rlrossiter
Copy link

This change adds enhancements to be able to use verifyOrder() and placeOrder() with keyNames that come from the API. There are also CLI enhancements to be able to do lookups to find packages, presets, categories, and items from the API, which are then fed into the verify/place_order functions.

Ryan Rossiter added 2 commits November 22, 2017 14:37
This change adds a new command, and 5 new subcommands:

slcli order - command for interacting with the ordering API

slcli order category-list -- subcommand for listing categories in a
package

slcli order item-list -- subcommand for listing items in a package

slcli order package-list -- subcommand for listing available packages

slcli order place -- subcommand for verifying/placing orders

slcli order preset-list -- subcommand for listing presets of a package

API functions in SoftLayer.managers.ordering.OrderingManager were also
added to programmatically interact with all of these functions as well.

The place_order() and verify_order() commands are built to pass a
package keyname, a location, and a list of item keynames. It then
transforms the keynames into IDs, which the place/verifyOrder APIs
accept.
The CLI appears to fail out if it gets AttributeError as part of any of
the manager calls, so in order to not make it look like a
softlayer-pythyon but, we need to spit out something from
SoftLayer.exceptions. The base exceptions.SoftLayerError was used.

Presets also needed to be handle differently. When listing the presets,
both the activePresets and accountRestrictedActivePresets need to be
retrieved and merged together before being returned.

Finally, extra doc was added to verify_order() and place_order() to
make it easier to understand how to use it.
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 84.37% when pulling 67a54fd on rlrossiter:order-enhancements into 0062380 on softlayer:master.

I updated names from price_keynames -> item_keynames, but I missed a
couple. Updated those to be consistent with item_keynames now.
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 84.37% when pulling 9526a96 on rlrossiter:order-enhancements into 0062380 on softlayer:master.

def get_preset_by_key(self, package_keyname, preset_keyname, mask=None):
"""Gets a single preset with the given key."""
preset_operation = '_= %s' % preset_keyname
_filter = {'activePresets': {'keyName': {'operation': preset_operation}}}
Copy link
Author

Choose a reason for hiding this comment

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

This is only looking in the active presets. It also needs to look at the account restricted active presets with this filter to possibly find it in there as well

Copy link
Author

Choose a reason for hiding this comment

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

turns out this filter works with both activePresets and accountRestrictedActivePresets, so nothing needs to be updated.

@allmightyspiff allmightyspiff self-requested a review December 1, 2017 20:15
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 85.812% when pulling 010f6f8 on rlrossiter:order-enhancements into 0062380 on softlayer:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 85.854% when pulling dd62296 on rlrossiter:order-enhancements into 0062380 on softlayer:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 86.069% when pulling 11ae799 on rlrossiter:order-enhancements into 0062380 on softlayer:master.

@allmightyspiff
Copy link
Member

$ ./slcli --really order place   --complex-type SoftLayer_Container_Product_Order_Hardware_Server --preset D2690_128GB_2X600GBSAS_RAID1_2  --extras '{"hardware":[{"hostname":"testORdering", "domain": "cgallo.com"}]}' BARE_METAL_SERVER TORONTO  1_IP_ADDRESS UNLIMITED_SSL_VPN_USERS_1_PPTP_VPN_USER_PER_ACCOUNT REBOOT_KVM_OVER_IP OS_UBUNTU_16_04_LTS_XENIAL_XERUS_64_BIT BANDWIDTH_0_GB_2 100_MBPS_PRIVATE_NETWORK_UPLINK
id       21469963
created  2017-12-15T15:41:31-06:00
status   PENDING_AUTO_APPROVAL

Noticed some oddness with locations though, I'm not sure why DAL13 is an invalid location here. Using any of the 3 letter dc names doens't seem to work.

$ ./slcli --really order place   --complex-type SoftLayer_Container_Product_Order_Hardware_Server --preset D2690_128GB_2X600GBSAS_RAID1_2  --extras '{"hardware":[{"hostname":"testORdering", "domain": "cgallo.com"}]}' BARE_METAL_SERVER DAL13 1_IP_ADDRESS UNLIMITED_SSL_VPN_USERS_1_PPTP_VPN_USER_PER_ACCOUNT REBOOT_KVM_OVER_IP OS_UBUNTU_16_04_LTS_XENIAL_XERUS_64_BIT BANDWIDTH_0_GB_2 100_MBPS_PRIVATE_NETWORK_UPLINK
SoftLayerAPIError(SoftLayer_Exception_Order_InvalidLocation): The location provided for this order is invalid.

Using its keyname seems to work though. Adding in a way to get the available DCs for a package might be nice.

$ ./slcli --really order place   --complex-type SoftLayer_Container_Product_Order_Hardware_Server --preset D2690_128GB_2X600GBSAS_RAID1_2  --extras '{"hardware":[{"hostname":"testORdering", "domain": "cgallo.com"}]}' BARE_METAL_SERVER DALLAS13 1_IP_ADDRESS UNLIMITED_SSL_VPN_USERS_1_PPTP_VPN_USER_PER_ACCOUNT REBOOT_KVM_OVER_IP OS_UBUNTU_16_04_LTS_XENIAL_XERUS_64_BIT BANDWIDTH_0_GB_2 100_MBPS_PRIVATE_NETWORK_UPLINK
SoftLayerAPIError(SoftLayer_Exception_Order_HardwareUnavailable): There is currently no hardware available for package # 200 with preset configuration # 101 in Dallas 13. Please select a different datacenter

Copy link
Member

@allmightyspiff allmightyspiff left a comment

Choose a reason for hiding this comment

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

Let me know your thoughts on the complexType thing and location comment. Even with those I'd be ready to merge this monday unless you want to make changes.

"""Utility functions for mapping packages and categories to complex types."""


def get_package_type_map():
Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to add the virtual server and bare metal server types here?

Copy link
Author

Choose a reason for hiding this comment

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

Apparently what I've been told, we can't map directly from that package to that order type

}


def get_category_type_map():
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how I feel about this level of hard coding categories.
Having the user figure out the complexType on their own might be preferable here. Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, my goal was to just make this as simple as possible for the user, because it's not obviously clear where to find the SoftLayer_Container_Product_Order type that needs to be used. Unfortunately, this was the only way to do that (there is no API call to properly get the type).

Maybe I should just move this out into a different PR that we can debate on (it'll thankfully make this change a little bit smaller). I'll make complexType a kwarg and CLI option so one day we'll be able to make it optional, but for now I'll require it to be set. How does that sound?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah thats a good plan.

@rlrossiter
Copy link
Author

Noticed some oddness with locations though, I'm not sure why DAL13 is an invalid location here. Using any of the 3 letter dc names doens't seem to work.

So the order API requires you to specify the full name for orders when you provide a location. I was thinking of doing a follow-up PR later to allow you to specify the short names for DCs as well.

"""
# verify the order, and if the order is valid, the proper prices will be filled
# into the order template, so we can just send that to placeOrder to order it
verified_order = self.verify_order(package_keyname, location, item_keynames,
Copy link
Author

Choose a reason for hiding this comment

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

verifyOrder() can strip data out of the original order in the returned order, so calling placeOrder with the verified order will be omitting some of the original order data. verify_order() and place_order() should then have a same base prepare_order() function that it then either calls verify or place on, depending on the function it is in.

Copy link
Member

@allmightyspiff allmightyspiff left a comment

Choose a reason for hiding this comment

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

unapproving for now based on our conversation.

Ryan Rossiter added 2 commits December 19, 2017 16:38
verifyOrder and placeOrder need to be called with the same generated
order dict, and the output of verifyOrder can't be sent to placeOrder,
since it strips out some extra data.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 86.55% when pulling 9d6c962 on rlrossiter:order-enhancements into 5f16b02 on softlayer:master.

@rlrossiter
Copy link
Author

@allmightyspiff I think this is now in a final working state if you want to take a look at it. Thanks!

Copy link
Member

@allmightyspiff allmightyspiff left a comment

Choose a reason for hiding this comment

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

looks good. I'll likely merge later this week

@allmightyspiff allmightyspiff merged commit a1e984a into softlayer:master Jan 5, 2018
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.

3 participants