Skip to content

Conversation

lmazuel
Copy link
Member

@lmazuel lmazuel commented Jun 30, 2014

Just a little commit to start the WebSpaces/WebSites feature (and make a not so big pull request :))
I have tried my best to respect your coding style.
I have made a few tests, but without "create websites" feature it's difficult to make something which works everywhere on every Azure account.

@huguesv
Copy link
Contributor

huguesv commented Jul 1, 2014

Thanks Laurent!

This is a good start. You made it really consistent with the rest of the code.

I just ran in a small issue. In test_list_web_spaces, the test fails because if temp.name == 'EastUSwebspace' is never true. The name that is returned is all lowercase. That seems to match the names listed here: http://msdn.microsoft.com/en-us/library/azure/dn236427.aspx

When new files are added, they need to be added to the 2 PTVS project files (.pyproj). If you can do it, that's great, otherwise I'll take care of it later.

Do you intend on adding more functionality to this API? If not, it may be best to put a doc comment on the class itself stating it is preliminary/incomplete and subject to change. Just because more than half of the API is not implemented and that may trigger breaking changes when we get to implement the remaining functionality.

Also, I want to acknowledge that we had a pull request for similar functionality before:
#120

Yours is slightly more correct when it comes to the desired factoring of functionality into a separate class. At the time I was hoping to have time to work on this, and implement the full API, but that didn't happen :(

@lmazuel
Copy link
Member Author

lmazuel commented Jul 2, 2014

Thank you for your answer :)

I re-tested the list_webspaces, and I don't understand why you have the name in lowercase. This is the exact XML I get:

<?xml version="1.0" encoding="UTF-8"?>
<WebSpaces xmlns="http://schemas.microsoft.com/windowsazure" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">
   <WebSpace>
      <AvailabilityState>Normal</AvailabilityState>
      <ComputeMode i:nil="true" />
      <CurrentNumberOfWorkers i:nil="true" />
      <CurrentWorkerSize i:nil="true" />
      <GeoLocation>BLU</GeoLocation>
      <GeoRegion>East US</GeoRegion>
      <Name>EastUSwebspace</Name>
      <NumberOfWorkers i:nil="true" />
      <Plan>VirtualDedicatedPlan</Plan>
      <Status>Ready</Status>
      <Subscription>XXXXXXXXXXXXXXXXXXXXX</Subscription>
      <WorkerSize i:nil="true" />
   </WebSpace>
</WebSpaces>

For the pyproj, I don't think it's a good idea for me to add it myself from my Ubuntu. I open it in textual mode to see, but my editor didn't understand the UTF-8 BOM and I'm not sure about what will result for endlines... I can tried another editor or a Windows VM if it helps a lot, but if it's easy for you, I think I will let you manage this :)

I don't think I will have time to complete the entire API. I'm going to add at least Metrics, but I'm not sure about something else (but not impossible, depending how our current project goes). I,M going to add some comments as you mentions.

Actually, I have seen #120 and I had take care about what you said in this :)

- Add "get webspace details"
- Add "get website details"
@huguesv
Copy link
Contributor

huguesv commented Jul 8, 2014

Thanks Laurent. I'm confused as to why we get different results. Your latest changes look good, I'll take the time to try them out tomorrow. Sorry for the delay, new urgent project got top priority + July 4th holiday.

huguesv added a commit that referenced this pull request Jul 8, 2014
List WebSpaces and WebSites feature
@huguesv huguesv merged commit a095d57 into Azure:master Jul 8, 2014
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.

2 participants