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

Promise support #110

Closed
abramz opened this issue Jan 11, 2016 · 33 comments
Closed

Promise support #110

abramz opened this issue Jan 11, 2016 · 33 comments

Comments

@abramz
Copy link

abramz commented Jan 11, 2016

Are you guys planning on supporting promises?

Currently, your callback function signatures do not lend themselves well to promisification as the callback is called with 2 arguments (result and response) on success. This requires developers to manually write a wrapper for all of the functions they want to promisify.

edit: I would be more than happy to contribute a PR, but want to know if this is in your road map and if you have opinions as to how it should be implemented.

@craxal
Copy link
Member

craxal commented Jan 11, 2016

Agreed. This would make the client API much nicer to code against.

@jamesdixon
Copy link
Contributor

This would be great!

@anildukkipatty
Copy link

Yes. More than happy to help.

@agilbert201
Copy link

Agree. Example of wrapper I used to get around this for now https://gist.github.com/agilbert201/de1fe06def2b3a344c1a

@jamesdixon
Copy link
Contributor

Not an expert on this by any means, but would promisification be possible using Bluebird's promisifyAll() with a custom promisifier? Seems like that's the purpose of the custom promisifier, but a little stuck on where to start.

@jamesdixon
Copy link
Contributor

This hasn't been updated in a while, but am liking the interface: https://github.com/tracker1/node-azure-storage-simple

@sethreidnz
Copy link

sethreidnz commented May 29, 2016

@jamesdixon I have tried to use promisifyAll on the returned object from azureStorage.createBlobService() and it doesn't work. It gives me an error about not passing a callback. I thought that bluebird handled non-promise aware APIs but perhaps this module is doing something in its parameter validation that means it doesn't work? Would love to see this possible

@abramz
Copy link
Author

abramz commented May 29, 2016

@JustSayNO did you use a custom promisfier?

@sethreidnz
Copy link

I used Bluebird.js? I'm not sure what you mean but custom but I just did what the documentation here sugggests:

http://bluebirdjs.com/docs/api/promise.promisifyall.html

When I have moment I'll post code sample.

@abramz
Copy link
Author

abramz commented May 30, 2016

As @jamesdixon said, you have to use a custom promisfier when you use promisifyAll. Azure calls the callback with 3 arguments, error, result, and response IIRC. This is not the node convention, and so you have to tell bluebird how to handle this.

@dongseok0
Copy link

@JustSayNO I use like this. https://gist.github.com/dongseok0/9312eacc2db52e2f8846d39c08088465

@ricklove
Copy link

ricklove commented Feb 24, 2017

I look forward to this being ready.

Here is my best workaround for Typescript which maintains autocomplete and intellisense support for arguments (which promisify breaks).

Update: I found a serious bug because I forgot to wrap the call in a try catch.

import { StorageError, ServiceResponse } from 'azure-storage';
import { createBlobService, BlobService, createQueueService, QueueService } from 'azure-storage';

export function asyncIt<T>(call: (serviceCallback: (error: StorageError, result: T) => void) => void): Promise<T> {
    return new Promise<T>((resolve, reject) => {
        try {
            call((error, result) => {
                if (error) { reject(error); }
                resolve(result);
            });
        } catch (err) {
            reject(err);
        }
    });
}

async function usage() {
    // --- Blobs ---
    const blobService = createBlobService();

    // Create a blob
    let blob = await asyncIt<BlobService.BlobResult>(cb => blobService.createBlockBlobFromText('container', 'blob', 'text', cb));

    // --- Queues ---
    const queueService = createQueueService();

    // Write a Message (No Return)
    await asyncIt(cb => queueService.createMessage('queue', 'text', cb));

    // Read a Message
    let message = await asyncIt<QueueService.QueueMessageResult>(cb => queueService.getMessage('queue', 'text', cb));
}

The main problem is having to declare return Types as the type inference cannot determine types from a nested callback argument.

@hasonmsft
Copy link

@ricklove , to support promise, there will be a breaking change. It's already in our backlog and we'll try to fix it in an appropriate time. Will get you updated when it's ready.

@ricklove
Copy link

ricklove commented Feb 24, 2017

@hasonmsft Ok, thanks!

Btw, why not just make an optional wrapper using something like the above?

It wouldn't change the existing interface, it would just be in a different module. In fact, I'm sure it could be automated easily enough and use the existing documentation. I don't so how it would have any negative effects on any existing consumers.

@alippai
Copy link

alippai commented Jan 26, 2018

Any news on this? Node.js supports async await now, so this would enable a really nice syntax to work with Azure storage.

@XiaoningLiu
Copy link
Member

We are planning Node.js client library v2 this year which will fully support promise style.

@anoff
Copy link

anoff commented Apr 6, 2018

Looking forward to that :)

@heyAyushh
Copy link

please please please

@Verikon
Copy link

Verikon commented May 23, 2018

Util.Promisify seems to work in my use case (Node >= 8)

import Azure from 'azure-storage';
import {promisify} from 'util';

let bls = Azure.createBlobService(account, key)
bls.getBlobToStream = promisify(bls.getBlobToStream).bind(bls);

@abramz
Copy link
Author

abramz commented May 23, 2018 via email

@DanaEpp
Copy link

DanaEpp commented Aug 3, 2018

So where are we on a new node library to support async/await/Promise ???

@Cellule
Copy link

Cellule commented Aug 3, 2018

promisify works great, but as mentioned you do lose SpeedSummary in some cases.
Also, because I want to keep using TypeScript and I want to keep intellisense when I'm using the API I had to write something like

  // Blob APIs
  getBlobMetadata(container: string, blob: string, options?: BlobService.BlobRequestOptions): Promise<BlobService.BlobResult> {
    return util.promisify(this.blobService.getBlobMetadata.bind(this.blobService))(container, blob, options);
  }
...
  // Queue APIs
  createQueueIfNotExists(queue: string): Promise<QueueService.QueueResult> {
    return util.promisify(this.queueService.createQueueIfNotExists.bind(this.queueService))(queue);
  }

which is super verbose and I just wish there was native support for promise from the module.
So far, every other azure module I've used have promise support

@seguler
Copy link

seguler commented Aug 10, 2018

Hey folks! We are currently working on a new Storage SDK for JS with Promise support! Stay tuned!

@seguler
Copy link

seguler commented Aug 17, 2018

Here is the PR for the new Storage SDK for JS: Azure/azure-storage-js#1

Looking forward to everyone's feedback!

@chriskuech
Copy link

Interesting that these docs manually add support for promises and describe the value of promises: https://docs.microsoft.com/en-us/azure/storage/blobs/storage-quickstart-blobs-nodejs

@XiaoningLiu
Copy link
Member

@chriskuech This doc is for legacy callback style azure-storage-node SDK, users need to wrap a promise based on the callbacks.

Please try with the @azure/storage-blob V10 SDK, which has native promise support.

@shavidzet
Copy link

Any update there?

@craxal
Copy link
Member

craxal commented Apr 1, 2020

@shavidzet The developers said that v10 SDK and up have native Promise support, and they're not going to be adding it to older versions. I've been using v12, and it works great.

@murraybauer
Copy link

Yes, but still awaiting Azure Table support in v10+

@AshUK
Copy link

AshUK commented Apr 21, 2020

Yeah we need table support..

@johnkors
Copy link

@XiaoningLiu Could you comment on Azure Table Storage support?

@nikolabebic95
Copy link

Agree. Example of wrapper I used to get around this for now https://gist.github.com/agilbert201/de1fe06def2b3a344c1a

I borrowed your code and modified it a bit, to create a TypeScript version of the azure table reader: https://gist.github.com/nikolabebic95/8f872396d92994493dec157dcc9d825b

@XiaoningLiu
Copy link
Member

Refer to https://www.npmjs.com/login?next=/package/@azure/data-tables for next generation table SDK : )

@EmmaZhu EmmaZhu closed this as completed Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests