-
Notifications
You must be signed in to change notification settings - Fork 26
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
Allow masks of all types to be added to a service #19
Comments
Some thoughts on a solution here. Instead of setting a mask (and filter?) on the service, instead you call a method on a service, which gives you a request object. On this request object, you set a properly-typed mask, filter, and result limit. The only downside is that there's an extra object that a user has to deal with, but I think it's a nice compromise because if the user doesn't care about the request, they can immediately send the request after calling the method. Account.Service service = Account.service(client);
Request request = service.getObject();
request.withMask().hardware();
Account account = request.send(); Ideally with the improvements to add filters and change the mask convention on #36, that can make for a 1.0.0 release. |
One of the original goals was to make the method invocations appear as idiomatic Java where an invocation was an invocation with results as though it was local. Everything else was added around this including things like async calls and metadata such as masks. Ideally things would stay that way and any meta data we could find somewhere else to put it. I personally still think at the service level is best and I don't see any benefit of moving the metadata down further since recreating a service is cheap and expected to happen for each different mask type. If you do go the per method route and you were ok with putting a Java 8+ restriction on the lib (it's too big for things like Android anyways), you can keep BC and just add a form of wrapper, e.g.: Account.Service service = Account.service(client);
// Of course would just use the diamond operator here, just being specific for explanation
Request<Guest> request = service.requestFor(service::getVirtualGuests);
specialized.withMask().virtualGuestNameOrWhatever();
request.send(); Basically just a lazy lamdba evaluated at |
For your first point of avoiding a request-type object to hold masks and filters, we could have services store masks/filters/etc per method I suppose (map method name -> mask, method name -> filter, etc), then only use the masks/filter/etc for the method called on the service. That would keep the idiomatic method calls. That could be made backwards compatible too (have default masks, filters, etc), but might be confusing. The second suggestion of wrapping the service for masking and filtering with BC is good, but then we sacrifice complexity (both requests and the service then have this extra data, one takes precedence over the other sometimes). Honestly I'm thinking a sacrifice of BC for a clearer convention is preferred. But perhaps other users of the client have other opinions :) |
As I'm no longer a user, I'll defer. Though if you are going to sacrifice BC, change the method names because idiomatic Java expects something that starts with a verb to do that thing. And although it's an internal decision, it may be worth asking yourselves the benefit vs level of effort required for these changes based on the popularity (or lack) of the project. |
Just for clarification, which method names are you referring to here? The changes here are for my own benefit (I like experimenting with the API with nice typing!) and the benefit of the people who seem to be using the client rather than any sort of internal decision. I'm mostly trying to see if we can get the client to a place where a bunch of helpful examples can be provided on https://softlayer.github.io and elsewhere when questions about how to do something come up. |
For example, right now |
Ah, I see what you mean. |
Currently only a mask of the type that is the same as the service can be added to a service. So if
Account.getObject
is called, it usesAccount.Mask
just fine, but ifAccount.getVirtualGuests
is called, it needs to be able to useGuest.Mask
. This is explained in http://sldn.softlayer.com/article/Object-Masks.It would probably also be wise to error if a wrong mask is about to be used on a method, but this could introduce a slight backwards incompatibility if people are not used to the error appearing and they are just using it wrong anyways, but we are still at a 0-based major version.
The text was updated successfully, but these errors were encountered: