-
Notifications
You must be signed in to change notification settings - Fork 4
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
Proposal: Add RSpec version to Allocate #47
base: master
Are you sure you want to change the base?
Conversation
This is very interesting and sounds like a problem worth addressing. A few thoughts: Perhaps at the crux of this is the fact that GetVersion indicates the schemas and namespaces for Request and Advertisement Rspecs but not Manifest RSpecs. It seems to be an implicit assumption that requests and manifests will be the same (i.e. 1:1 corresponding) schema. I think that is in general a reasonable assumption: I'm hard-pressed to picture a scenario in which you'd want a different schema of Manifest returned. But it might be good to add this manifest version info into the GetVersion return to make the mapping more explicit. I am not sure the additional Allocate argument is really value-added. the RSpec is just XML and thus it is a simple string search for the schema. We're pushing the burden from aggregates onto tools and I'm not sure it is really worth it. But I don't object.
On Jul 1, 2014, at 10:16 AM, Wim Van de Meerssche wrote:
Marshall Brinn |
Yes, the 1:1 mapping between request RSpec and manifest RSpec is indeed an assumption. This is mentioned at GetVersion:
And in the section about the Allocate return value:
This should also be repeated clearly in the Rspec Document. (the "contract" mentioned above is not fully in there yet). The 1:1 mapping is not a bad assumption, as it makes things simple, but ... note that the Geni RSpec v3 manifest RSpec has the same namespace as the request RSpec, but NOT the same schema. There are also additional assumption made here:
Are these assumptions acceptable limitations? If these are acceptable, we can put these assumptions explicitly in the document, and then we do not need any additional parameters to Allocate. If either of these assumptions is restricting the API too much, so if the API should also allow non-XML RSpecs, it might be best to tag at least the request RSpec with its version. This would be similar to what is done for the credentials. (An extra argument for the format of the returned manifest RSpec is not necessary in my opinion. I've put it in the initial proposal to show how far this could go.) |
HI Wim - Personally, I think the assumptions are acceptable limitations. I think that it will be very difficult to write tools if the RSpecs are of arbitrary format. I agree these things should be made much more clear and explicit in the document. Hmmm... Sounds to me like the fact that RSpecs and Manifests have the same namespace but not the same schema is broken and should be fixed. Cheers - Marshall On Jul 3, 2014, at 2:49 AM, Wim Van de Meerssche wrote:
Marshall Brinn |
Context:
The following calls return an RSpec:
The following calls take an RSpec as argument:
Problem:
Allocate is the only one of the calls that returns an RSpec, but does not have the mandatory rspec_version argument that specifies in which format it needs to be returned. (It does not even have this argument as an option.)
Additionally, Allocate takes a request RSpec as mandatory parameter, but users cannot specify in which format this request RSpec is. So I guess the server needs to figure this out. This makes it harder for servers that supports multiple request RSpec formats: they needs to be able to determine the type of the RSpec. Assuming RSpecs are always XML, this might be easy (using the namespace), but that is perhaps not always a valid assumption?
Solution:
I propose to add the rspec_version argument to Allocate, for both the argument RSpec and the return RSpec. Is this a good idea? Any feedback on this?
Alternative solution
Note that AMv3 does not specify how the AM detects the request manifest type, but it does specify that the AM should reply with a manifest RSpec in the same format as the request RSpec. So another option is to specify only the request RSpec type, and return a manifest of the same RSpec type.
This probably isn't easier to implement however, as the AM is still required to translate the manifest RSpec if the client issues a Describe call and requests another format.
How to change the API:
This issue contains a proposal on how to change the API to include both RSpec versions. This is meant to demonstrate how this could be done. It will be updated based on the result of discussing the above.
(If this proposal is a good idea, it might be added to the API in a cleaner way, so any feedback on that is also appreciated! For example, argument 3, the "struct rspec" now contains both the type, version and the rspec itself. Would it be better to split this up? Also, the manifest_rspec_type could also be an option instead of a mandatory argument, and when not specified, it could default to the same version as the request. )