Merge lp://qastaging/~justin-fathomdb/nova/add-xsd into lp://qastaging/~hudson-openstack/nova/trunk

Now *this* is what metadata is.

I hope you've reviewed it in detail Jay, knowing how much you love XML :-)

We've had a couple of issues with typing of data and some questions over XML
support, so I think importing the existing XML schema is the right first
step. We'll probably have to create a second XML schema for OpenStack v1.1
to the extent that it's different. I'm going to bring this up in the "any
other business" at the end of our weekly meeting.

I was able to generate a JAXB Java binding to this schema, so I think it's complete now...

We shouldn't use rackspace product names in the XSD...

 * Cloud Servers -> OpenStack Compute or Nova?
 * cloudServersFault -> computeFault

...

True, but this is for compatibility with v1.0.

I am on the lookout for an OpenStack-ified v1.1 XSD, if anyone sees one
hanging around...

If you want this into Cactus rather than wait for Diablo, could you provide a quick benefit vs. risk of regression rationale, then request a specific FFe review from me ? This looks rather self-contained so I think it should be able to get in.

Sure! Technically though, this one isn't subject to FF, because this is not a Feature, this is to support a bugfix for Bug #740576. It was linked, but 'someone' unlinked it ;-) The bug is that our XML API isn't to spec. This is the spec :-)

Risks:
I do believe there's as close to zero risk as is possible - these files aren't even referenced. If they are referenced in future, they'll likely initially be referenced only by tests (validating our XML against the schema). If they are referenced by non-tests, it will be because it's the most efficient way to deliver a correct XML API.

Benefits:
By having the spec in the codebase, we're able to validate against it.

I was planning on writing tests using the Java bindings.

I don't like the idea of adding another root dir, can we dump these under the nova/api/openstack dir?

Fine with me, except we might want to have schemas for other APIs (EC2?), or
even schema for internal protocols (e.g. the glance protocol, or maybe our
message format in the multi-zones case).

How about nova/api/schema or nova/schema?

The downside of putting them under nova is that they're not code (well, not
Python code).

my thoughts on this are that I am trying to move for better encapsulation of the concerns of the sections of the project, with some hope of being able to physically separate them in the future, so if there are schemas specific to the implementation of this API I think they should live under it.

Agreed this is part of the bugfix and doesn't require FFe. For clarity I'll set an approve here.

i'll take your word for it that all this gibberish makes sense ;) I'm happy enough with the location of it.

i admit. i didn't read every piece of this. :)

Now, if this was accompanied by some tests that actually used it, I could see its value. As is, it's an XML Schema that *something else* (the existing Rackspace Cloud Servers) validates against. I think having this in the tree while we're not even sure we validate against it is confusing, and I don't think I understand its value.

If you can add some tests that actually use it, I'd be happy to accept it even much later in the cycle. Alternativaly, (much less idealy) if you could document that you've used some external tool to validate our stuff against this schema and it actually passes, I think it'll make much mose sense to have this in the tree as some kind of documentation.

Hi Soren - it's a fair concern. I was trying to keep my patches fairly
self-contained; I originally proposed in a separate patch incorporating a
Java binding which actually uses this schema, but it looks like that doesn't
belong in core. I don't really have experience of using schemas in Python.

The fact is, we are supposed to be supporting this API. This is the spec
for our "v1.0" endpoint. While XML can be fugly, it does have the benefit
of being an excellent spec for the wire format, which it's easy to test
against. Most of the specs carry across to our JSON output. If we want our
API to be compatible with the many existing tools out there that talk to the
CS API, this is the spec we have to validate against.

I'm also trying to track down the v1.1 bindings, for exactly the same
reason. However, apparently they might still be in flux...?

I did originally have tests, but it looks like someone that knows Python +
XML better is going to have to do it for language reasons. My tests did
find that we're not even putting in a namespace declaration, so everything
failed immediately. I'm working on the fix for that in the parallel branch.
 Once the namespace declaration is included, basic functionality works, but
that's as far as my testing got so far. The discussion on the namespace
branch also showed up additional problems with our output (JSON & XML) when
it comes to limits. So we're drawing benefits from this work even now, and
I think in core we'll derive even more.

I think the right thing to do is to merge these specs, on the ground that
they are the best specs we've got for what we're supposed to be targeting.
 Even if we don't have tests immediately, it's helpful to have this in our
source tree, rather than on some website. Other languages can use this to
test our API, and hopefully someone that knows Python can e.g. add an output
filter to validate the XML as it is returned or add it to our unit
tests. The first step is to add these schemas though - otherwise we'll
never break the chicken & egg stalemate.

2011/3/31 justinsb <email address hidden>:
> The fact is, we are supposed to be supporting this API.

Right. So this should be referenced from a blueprint or something.

Documentation should document how things *are*, not how we wish they were.

> This is the spec
> for our "v1.0" endpoint.  While XML can be fugly, it does have the benefit
> of being an excellent spec for the wire format, which it's easy to test
> against.  Most of the specs carry across to our JSON output.  If we want our
> API to be compatible with the many existing tools out there that talk to the
> CS API, this is the spec we have to validate against.

I completely agree, so if these schemas were used for this validation,
I'd be perfectly happy with it.

> I think the right thing to do is to merge these specs, on the ground that
> they are the best specs we've got for what we're supposed to be targeting

Hmm... I got cut off, somehow..

> I think the right thing to do is to merge these specs, on the ground that
> they are the best specs we've got for what we're supposed to be targeting.

"supposed to be" are the operative words for me here.

If you would stick the files in a directory named
"schemas_that_we_ought_to_be_following_but_are_not",
"wishful_thinking" or even "somebody_elses_schema" that would help
immensely. :)

> Now, if this was accompanied by some tests that actually used it, I could see
> its value. As is, it's an XML Schema that *something else* (the existing
> Rackspace Cloud Servers) validates against. I think having this in the tree
> while we're not even sure we validate against it is confusing, and I don't
> think I understand its value.
>
> If you can add some tests that actually use it, I'd be happy to accept it even
> much later in the cycle. Alternativaly, (much less idealy) if you could
> document that you've used some external tool to validate our stuff against
> this schema and it actually passes, I think it'll make much mose sense to have
> this in the tree as some kind of documentation.

Absolutely agree. Adding this right now is just bloat IMO and doesn't address the issue/bug. I'd rather see it added a little a time with corresponding tests rather than get added all at once with no tests.

I totally agree that we should have been validating against this since day 1, and that we should have tests that pass.

We didn't do that, and everything is broken. Let's move forwards.

This is the best schema we've got for the OpenStack API (JSON or XML). There's a parallel branch that makes validation not fail completely. There was a test branch that wasn't wanted in core because it used Java, so was rejected. If I write a test, it'll fail, which will mean it can't be merged, because I tried proposing tests that "fail due to known bugs" and that was rejected. If I sit down for two hours and fix the XML output in one big patch that fixes everything, then the patch will be too big and will be rejected. So I have to do it one step at a time, over the course of two releases instead of two hours. Fine. But let's at least take the first step.

Vish: as PTL, can you please make the call here.

Wouldn't it be possible to merge just the "flavors" XSD and tests for flavor XML responses? Then move to images, servers, and so on? I agree that it won't get merged if you do one big patch which tests everything, but piecemeal is way to go here IMO.

Now that the blocking branch is going in I think we can take the step-by-step approach. Btw, validation in python is pretty simple:

    schema = etree.XMLSchema(file=os.path.join(os.path.dirname(__file__),
                                               '../../schema/nrml.xsd'))
    parser = etree.parse(StringIO(str(xml)))
    if not schema.validate(parser):
        raise exceptions.ValidationError("Invalid NRML document.")

I have a feeling that as we will be running into bugs in each section, so this will keep it manageable.

On Apr 6, 2011, at 10:45 AM, Brian Lamar wrote:

> Wouldn't it be possible to merge just the "flavors" XSD and tests for flavor XML responses? Then move to images, servers, and so on? I agree that it won't get merged if you do one big patch which tests everything, but piecemeal is way to go here IMO.
> --
> https://code.launchpad.net/~justin-fathomdb/nova/add-xsd/+merge/54418
> You are requested to review the proposed merge of lp:~justin-fathomdb/nova/add-xsd into lp:nova.

My preferred option is one mega-patch which just adds the XML schema and
fixes all the output to be schema-compliant; that's probably about 2 hours
work. The way we're doing it, as a sequence of atomic patches and debating
over each one, we can stretch that out over 2 releases. But I think
breaking this up further into even smaller patches is a bad idea if we want
well-formed output this year.

Schemas are allowed to cross-reference each other, so it's not clear a
priori that we can do so. I don't really see the upside in doing so.

Vish - messages "crossed in the mail"! Thanks for that example; I'll take
that code and add warnings whenever we output non-compliant XML. I'll do
that in a derived branch from this one.

Warnings on non-compliance are a great idea, and might even be enough to convince Soren and Brian that this should go in!

On Apr 6, 2011, at 11:03 AM, justinsb wrote:

> Vish - messages "crossed in the mail"! Thanks for that example; I'll take
> that code and add warnings whenever we output non-compliant XML. I'll do
> that in a derived branch from this one.
>
> --
> https://code.launchpad.net/~justin-fathomdb/nova/add-xsd/+merge/54418
> You are requested to review the proposed merge of lp:~justin-fathomdb/nova/add-xsd into lp:nova.

Validation of XML output against this schema is now available in the derived branch:
https://code.launchpad.net/~justin-fathomdb/nova/check-xsd/+merge/56658

Small governance note:

Though the Nova PTL can make a final call on difficult decisions, he is not there to bypass the normal nova-core branch review process, and should be called as a last measure only. He shouldn't overrule the nova-core decision, but only have a casting vote in the case nova-core members can't reach an agreement. Otherwise let's just say Vish reviews branches and give other nova-core members some vacation.

Sorem, Brian:

Do you still feel this is a disapprove now that we have a branch with tests? I'm leaning toward getting this in so we can start fixing the issues that the tests expose. I'm not concerned about this making it into cactus specifically, but it seems like there is a reason for this to be there now.

Vish, I agree this could help us expose and squash a good number of API-formatting-related bugs but in my opinion it's a feature branch and thus needs to wait until Diablo opens up. Also I believe that Justin and Brian Waldon have agreed we should talk a lot of this out at the dev conference.

I'm a disapprove for Cactus still, but I'm more than ready to get better testing frameworks/methods in place for future releases.

I think we should discuss all of this at the design conference, but
the schema that we're supporting should have been part of our test
suite on day 1. Let's fix that in Cactus, not in D/E/F.

Right now, we only have one usable API, and it's EC2. I think getting
a working CloudServers / OS v1.0 API is worth delaying Cactus over
alone, never mind the many other issues.

As long as we keep this as an internal checking thing, then it's fine. If it ends up being something people might consider documentation, it's not so fine. I'm very, very tired right now, so can't really judge either way. I'll look at it tomorrow when I'm capable of more coherent thought.

I'm cool with this. And yes, Justin, even though I'm not a huge fan of XML, I do see the value of this ;)

Who is the owner of the schema? Is it Rackspace or OpenStack?

For 1.0 it seems it's Rackspace. Since it's a Rackspace resource I think we should just point the documentation to the Rackspace site with a note "You can get the XSD here [URL]". Just like the the RS API docs live on the RAX site.

Later, when it's an OpenStack API, under our control, it could get brought under our source control (with tests).

$0.02