Skip to content
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

[Ready for review] Support for streaming created resources #1300

Merged
merged 10 commits into from
Dec 7, 2016
Merged

[Ready for review] Support for streaming created resources #1300

merged 10 commits into from
Dec 7, 2016

Conversation

anuchandy
Copy link
Member

Today the Observable from createAsync emits only the root resource not any of the dependent resources that gets created.

A new method createAsyncStreaming with following signature has been added to address this

    /**
     * Puts the request into the queue and allow the HTTP client to execute
     * it when system resources are available.
     *
     * @param enableStreaming true if the resources needs to be emitted in the order
     *                        they gets created, false if only the root resource needs
     *                        be streamed
     * @return an observable where the resource and all it's dependencies gets emitted
     */
    Observable<Indexable> createAsyncStreaming(boolean enableStreaming);

if enableStreaming is true then caller will receive the resources as they gets created otherwise only the root resource will be emitted. The an example usage is:

        VirtualMachine virtualMachine = (VirtualMachine) virtualMachines.define(vmName)
                .withRegion(Region.US_EAST)
                .withNewResourceGroup(rgCreatable)
                .withNewPrimaryNetwork("10.0.0.0/28")
                .withPrimaryPrivateIpAddressDynamic()
                .withNewPrimaryPublicIpAddress(ResourceNamer.randomResourceName("pip", 20))
                .withPopularWindowsImage(KnownWindowsVirtualMachineImage.WINDOWS_SERVER_2012_R2_DATACENTER)
                .withAdminUsername("testuser")
                .withAdminPassword("12NewPA$$w0rd!")
                .withSize(VirtualMachineSizeTypes.STANDARD_D1_V2)
                .withNewStorageAccount(storageCreatable)
                .withNewAvailabilitySet(ResourceNamer.randomResourceName("avset", 10))
                . createAsyncStreaming(true)
                .map(new Func1<Indexable, Resource>() {
                    @Override
                    public Resource call(Indexable resource) {
                        Resource createdResource = (Resource) resource;
                        System.out.println("Created :" + createdResource.id());
                        return createdResource;
                    }
                }).toBlocking().last();

Why not streaming as Resource: There are couple of types those are creatable but not Resource, the same applicable with HasId as well

some resources that belongs to this category are:

azure-mgmt-graph-rbac/src/main/java/com/microsoft/azure/management/graphrbac/User.java
azure-mgmt-resources/src/main/java/com/microsoft/azure/management/resources/Deployment.java

@martinsawicki
Copy link

what is the boolean param needed for, if createAsync() already does what createAsyncStreaming(false) presumably would do? I'd think it'd be parameterless (at least the public function). Am i missing something?

@anuchandy
Copy link
Member Author

@martinsawicki you are right, will remove the flag

@martinsawicki
Copy link

if @jianghaolu signs off on this PR, I'm set. Thanks

@martinsawicki
Copy link

btw, is the Observable from createAsync any more useful than Future? If not, should we consider reverting to returning Future from createAsync, and dedicate cerateAsyncStreaming to the full power of observables?

@martinsawicki
Copy link

martinsawicki commented Dec 3, 2016

another thought wrt naming: i'm wondering now if createAsyncReactive() would be more descriptive/suggestive name for what it does than createAsyncStreaming()?

@jianghaolu
Copy link
Contributor

We heavily depend on the Observable<> createAsync() ourselves so no we can't remove that.

Now that I think about it a little more, I feel a little more comfortable about replacing our current version of createAsync() with this "streaming" one. People who are entry level users can use the Future one to just get the VM and for the Observable one, they can always call .last() on the Observable to just get the top level resource.

@anuchandy
Copy link
Member Author

yes - vmCreatable.CreateAsync().last() for the observable that emit just the top level vm, provided they have to map it to VirtualMachine in case vm needs to be used in down the line, which seems ok to me.

Observable<VirtualMachine> vmObservable = vmCreatable
    .CreateAsync().last()
    .map(new Func1<Indexable, VirtualMachine>() {
       public VirtualMachine call(Indexable r) {
         return r;
       }
    });

vmObservable.flatMap(new Func<VirtualMachine, Observable<?>>() {
  public ? call(VirtualMachine vm) {
    // use vm
    // return an observable<?> represents next async op
  }
})

@martinsawicki
Copy link

yeah, i'd be for the simpler/more aggressive approach (just one createAsync()). We're betting on Observables, might as well put that front and center. We'll just want to be clear in the samples/docs about the usage, but I really think ultimately the granular real-time progress info is one of the main reasons devs who use the async will gravitate toward that pattern. It's more complex to learn/use but it gives you all the info you can wish for.

@anuchandy
Copy link
Member Author

@martinsawicki @jianghaolu please review when you get a chance.

@anuchandy anuchandy changed the title Support for streaming created resources [Ready for review] Support for streaming created resources Dec 7, 2016
@anuchandy anuchandy merged commit 147cc82 into Azure:master Dec 7, 2016
@anuchandy anuchandy deleted the resource-stream-2 branch February 8, 2017 23:47
jianghaolu pushed a commit to jianghaolu/azure-sdk-for-java that referenced this pull request Feb 27, 2017
[Ready for review] Support for streaming created resources
jianghaolu pushed a commit to jianghaolu/azure-sdk-for-java that referenced this pull request Feb 27, 2017
[Ready for review] Support for streaming created resources
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.

4 participants