akamai / NetStorageKit-Node

Akamai NetStorage API for Node
Apache License 2.0
20 stars 15 forks source link

Full support for all additional action header values on dir and list APIs #2

Closed bobbyjball3 closed 7 years ago

bobbyjball3 commented 7 years ago

After using my own rewrite of list API call and discovering the difference in behavior with dir, it's clear that full support for all of the action headers for dir _(e.g. prefix, start, maxentries, etc.) and list are needed for customers like myself with huge numbers of objects in NetStorage buckets and directories.

list already accepts an object with the max_entries action header from my previous pull request. But we should probably add support to dir and make the allowable action headers more feature-complete with the NS API. We'll have to be careful not to break functionality against NS3 though since some of the supported entries differ between NS3/4.

bobbyjball3 commented 7 years ago

I'll add that I'm happy to fork and work on this since I've already modified some of the code in a local repo for my own testing.

AstinCHOI commented 7 years ago

Dear, @bobbyjball3

Thanks for your idea and test. Please comment it after testing ! I really appreciate your contribution.

bobbyjball3 commented 7 years ago

@AstinCHOI Great. I'll mock something up and test tomorrow.

bobbyjball3 commented 7 years ago

@AstinCHOI For the dir and list netstorage methods, I'm considering something like the following. By doing this, though it's more complicated, we maintain compatibility for just passing the path instead of verbose options. Since most people won't need verbose options, this keeps the usability high, while allowing for more advanced users to use the more advanced options.

dir(opts, callback) {
  if (typeof opts === 'object') {
    // set options from object
  } else if (typeof opts === 'string') { 
    // set path from string
  } else {
    // throw an error here because we couldn't sort it out
  }

  //actually make request
}
AstinCHOI commented 7 years ago

@bobbyjball3 It looks good !

bobbyjball3 commented 7 years ago

@AstinCHOI I've got this functionality added in my most recent branch. All tests pass, and I'll be submitting a PR soon.

Would you mind assigning this issue to me and adding the enhancement tag?

bobbyjball3 commented 7 years ago

I believe that PR 3 should close out this issue out once reviewed.

AstinCHOI commented 7 years ago

I really appreciate your contribution !