Issue 2412041: Google finance support for googlecl
|
Created:
15 years, 8 months ago by Ed Bartosh Modified:
2 years, 7 months ago Reviewers:
thmiller CC:
googlecl-dev_googlegroups.com Visibility:
Public. |
Patch Set 1 #
Total comments: 42
Patch Set 2 : Google finance support for googlecl (2nd attempt) #
Total comments: 2
Patch Set 3 : Google finance support for googlecl (3rd attempt) #
Total comments: 3
Patch Set 4 : Google finance support for googlecl (4th attempt) #
MessagesTotal messages: 9
Good start! I wasn't sure if you wanted to check in incrementally for finance support or get it all in one go. If you want to do it incrementally, focus on the use of the BaseCL methods (GetEntries, DeleteEntries) and don't worry about the --fields and EntryToString wrappers. http://codereview.appspot.com/2412041/diff/1/src/google.py File src/google.py (right): http://codereview.appspot.com/2412041/diff/1/src/google.py#newcode701 src/google.py:701: ' Finance only - transaction creation date') Did you thoroughly test all the calendar tasks after changing the default? I think it should be ok, but I'm not sure. http://codereview.appspot.com/2412041/diff/1/src/google.py#newcode764 src/google.py:764: help=("Finance only - specify notes for transaction")) Can you add these in alphabetized by --long-name? OCD-lightful. (Though I just noticed that the d's are out of order. Bah. Still, the request stands.) http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py File src/googlecl/finance/service.py (right): http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:89: """Get portfolio by title.""" Can you describe the arguments and return value? The style is inconsistent because I've been lazily updating the docs, but it should be: """Short desc. Longer desc if necessary. Args: arg1: type Description of parameter Returns: What return value means (and type). http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:95: return pfl Did you look at using googlecl.base.BaseCL.GetEntries() to do this? If you only need exactly one portfolio, use GetSingleEntry(). http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:102: """Create transaction.""" Documentation. (see above) http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:107: date = datetime.datetime.strptime(date, DATE_FORMAT) This can throw a ValueError if date does not match DATE_FORMAT. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:141: # Portfolio What does this comment mean? http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:162: print fmt % ('Id', 'Title', 'Curr', 'Gain', 'Gain %', 'C.basis', You should let the user decide what fields to list with the --fields option. picasa/service.py and contacts/base.py contain pretty extensive wrapper classes to base yours off of, or you can choose another way to do it. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:232: client.DeleteTransaction(transaction) Take a look at DeleteTransaction. I bet we can use DeleteEntry instead, which is sometimes helpful. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:237: pfl = client.get_portfolio(options.title) Is a portfolio similar to a blog, where the user will most likely only have one of them? Or is it more like an album in Picasa? http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:284: required = ['title', 'ticker', 'txnid']), Is there any shorthand for something like "list-transactions" that makes sense? That's an awfully long task name. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:292: # End: What are these comments for? I've never seen them, but they look potentially useful.
Sign in to reply to this message.
Hi Tom, Thank you for the review. Should I create new issue when I'm done whith the changes or it's possible to reupload new patch to this one? http://codereview.appspot.com/2412041/diff/1/src/google.py File src/google.py (right): http://codereview.appspot.com/2412041/diff/1/src/google.py#newcode701 src/google.py:701: ' Finance only - transaction creation date') On 2010/10/09 21:17:03, thmiller wrote: > Did you thoroughly test all the calendar tasks after changing the default? I > think it should be ok, but I'm not sure. I only looked at the calendar code. OK, I'll lool what I can do. http://codereview.appspot.com/2412041/diff/1/src/google.py#newcode764 src/google.py:764: help=("Finance only - specify notes for transaction")) On 2010/10/09 21:17:03, thmiller wrote: > Can you add these in alphabetized by --long-name? OCD-lightful. (Though I just > noticed that the d's are out of order. Bah. Still, the request stands.) OK, I'll do that. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py File src/googlecl/finance/service.py (right): http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:89: """Get portfolio by title.""" On 2010/10/09 21:17:03, thmiller wrote: > Can you describe the arguments and return value? The style is inconsistent > because I've been lazily updating the docs, but it should be: > """Short desc. > > Longer desc if necessary. > > Args: > arg1: type Description of parameter > > Returns: > What return value means (and type). OK, I'll do that. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:95: return pfl On 2010/10/09 21:17:03, thmiller wrote: > Did you look at using googlecl.base.BaseCL.GetEntries() to do this? If you only > need exactly one portfolio, use GetSingleEntry(). I'll try. Thank you for the suggestion. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:102: """Create transaction.""" On 2010/10/09 21:17:03, thmiller wrote: > Documentation. (see above) Will do. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:107: date = datetime.datetime.strptime(date, DATE_FORMAT) On 2010/10/09 21:17:03, thmiller wrote: > This can throw a ValueError if date does not match DATE_FORMAT. I thought it's checked somewhere before this code is called. Don't you think it would make sense considering that --date is used in many services? http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:141: # Portfolio On 2010/10/09 21:17:03, thmiller wrote: > What does this comment mean? Portfolio-related functions. There are also position and transactin-related functions below. Is it ok if I change this to 'Portfolio-related functions'? http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:162: print fmt % ('Id', 'Title', 'Curr', 'Gain', 'Gain %', 'C.basis', On 2010/10/09 21:17:03, thmiller wrote: > You should let the user decide what fields to list with the --fields option. > picasa/service.py and contacts/base.py contain pretty extensive wrapper classes > to base yours off of, or you can choose another way to do it. OK. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:232: client.DeleteTransaction(transaction) On 2010/10/09 21:17:03, thmiller wrote: > Take a look at DeleteTransaction. I bet we can use DeleteEntry instead, which is > sometimes helpful. OK http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:237: pfl = client.get_portfolio(options.title) On 2010/10/09 21:17:03, thmiller wrote: > Is a portfolio similar to a blog, where the user will most likely only have one > of them? Or is it more like an album in Picasa? It's more like album. Portfolio is a set of shares. Some investors can have several portfolios. For example long-term and short term or industry or country-specific portfolios. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:284: required = ['title', 'ticker', 'txnid']), On 2010/10/09 21:17:03, thmiller wrote: > Is there any shorthand for something like "list-transactions" that makes sense? > That's an awfully long task name. I've seen txn used as a shorthand for transaction. However it will be non consistent to use 'position'. How about 'pos' and 'txn' shorthands then? http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:292: # End: On 2010/10/09 21:17:03, thmiller wrote: > What are these comments for? I've never seen them, but they look potentially > useful. They're for Emacs. You can delete them if you want. I put them there because your code style is different from what I'm using for Python code. Is it Google coding style?
Sign in to reply to this message.
You can submit the new patch to the same issue. I think the option is -i #### if you're using rietveld's upload.py, not sure how you do it through the web interface. - Tom On Sat, Oct 9, 2010 at 3:34 PM, <bartosh@gmail.com> wrote: > Reviewers: thmiller, > > Message: > Hi Tom, > > Thank you for the review. > Should I create new issue when I'm done whith the changes or it's > possible to reupload new patch to this one? > > > > http://codereview.appspot.com/2412041/diff/1/src/google.py > File src/google.py (right): > > http://codereview.appspot.com/2412041/diff/1/src/google.py#newcode701 > src/google.py:701: ' Finance only - transaction creation date') > On 2010/10/09 21:17:03, thmiller wrote: >> >> Did you thoroughly test all the calendar tasks after changing the > > default? I >> >> think it should be ok, but I'm not sure. > > I only looked at the calendar code. OK, I'll lool what I can do. > > http://codereview.appspot.com/2412041/diff/1/src/google.py#newcode764 > src/google.py:764: help=("Finance only - specify notes for > transaction")) > On 2010/10/09 21:17:03, thmiller wrote: >> >> Can you add these in alphabetized by --long-name? OCD-lightful. > > (Though I just >> >> noticed that the d's are out of order. Bah. Still, the request > > stands.) > OK, I'll do that. > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py > File src/googlecl/finance/service.py (right): > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:89: """Get portfolio by title.""" > On 2010/10/09 21:17:03, thmiller wrote: >> >> Can you describe the arguments and return value? The style is > > inconsistent >> >> because I've been lazily updating the docs, but it should be: >> """Short desc. > >> Longer desc if necessary. > >> Args: >> arg1: type Description of parameter > >> Returns: >> What return value means (and type). > > OK, I'll do that. > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:95: return pfl > On 2010/10/09 21:17:03, thmiller wrote: >> >> Did you look at using googlecl.base.BaseCL.GetEntries() to do this? If > > you only >> >> need exactly one portfolio, use GetSingleEntry(). > > I'll try. Thank you for the suggestion. > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:102: """Create transaction.""" > On 2010/10/09 21:17:03, thmiller wrote: >> >> Documentation. (see above) > > Will do. > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:107: date = > datetime.datetime.strptime(date, DATE_FORMAT) > On 2010/10/09 21:17:03, thmiller wrote: >> >> This can throw a ValueError if date does not match DATE_FORMAT. > > I thought it's checked somewhere before this code is called. Don't you > think it would make sense considering that --date is used in many > services? > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:141: # Portfolio > On 2010/10/09 21:17:03, thmiller wrote: >> >> What does this comment mean? > > Portfolio-related functions. There are also position and > transactin-related functions below. > Is it ok if I change this to 'Portfolio-related functions'? > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:162: print fmt % ('Id', 'Title', 'Curr', > 'Gain', 'Gain %', 'C.basis', > On 2010/10/09 21:17:03, thmiller wrote: >> >> You should let the user decide what fields to list with the --fields > > option. >> >> picasa/service.py and contacts/base.py contain pretty extensive > > wrapper classes >> >> to base yours off of, or you can choose another way to do it. > > OK. > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:232: > client.DeleteTransaction(transaction) > On 2010/10/09 21:17:03, thmiller wrote: >> >> Take a look at DeleteTransaction. I bet we can use DeleteEntry > > instead, which is >> >> sometimes helpful. > > OK > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:237: pfl = > client.get_portfolio(options.title) > On 2010/10/09 21:17:03, thmiller wrote: >> >> Is a portfolio similar to a blog, where the user will most likely only > > have one >> >> of them? Or is it more like an album in Picasa? > > It's more like album. Portfolio is a set of shares. Some investors can > have several portfolios. For example long-term and short term or > industry or country-specific portfolios. > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:284: required = ['title', 'ticker', > 'txnid']), > On 2010/10/09 21:17:03, thmiller wrote: >> >> Is there any shorthand for something like "list-transactions" that > > makes sense? >> >> That's an awfully long task name. > > I've seen txn used as a shorthand for transaction. However it will be > non consistent to use 'position'. How about 'pos' and 'txn' shorthands > then? > > http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... > src/googlecl/finance/service.py:292: # End: > On 2010/10/09 21:17:03, thmiller wrote: >> >> What are these comments for? I've never seen them, but they look > > potentially >> >> useful. > > They're for Emacs. You can delete them if you want. I put them there > because your code style is different from what I'm using for Python > code. Is it Google coding style? > > > > Please review this at http://codereview.appspot.com/2412041/ > > Affected files: > M setup.py > M src/google.py > A src/googlecl/finance/__init__.py > A src/googlecl/finance/service.py > > >
Sign in to reply to this message.
Hi Tom, I'm done with the fixes. Please, review my comments and the code. Thank you, Ed http://codereview.appspot.com/2412041/diff/1/src/google.py File src/google.py (right): http://codereview.appspot.com/2412041/diff/1/src/google.py#newcode701 src/google.py:701: ' Finance only - transaction creation date') On 2010/10/09 21:17:03, thmiller wrote: > Did you thoroughly test all the calendar tasks after changing the default? I > think it should be ok, but I'm not sure. Done. http://codereview.appspot.com/2412041/diff/1/src/google.py#newcode764 src/google.py:764: help=("Finance only - specify notes for transaction")) On 2010/10/09 21:17:03, thmiller wrote: > Can you add these in alphabetized by --long-name? OCD-lightful. (Though I just > noticed that the d's are out of order. Bah. Still, the request stands.) Done. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py File src/googlecl/finance/service.py (right): http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:89: """Get portfolio by title.""" On 2010/10/09 21:17:03, thmiller wrote: > Can you describe the arguments and return value? The style is inconsistent > because I've been lazily updating the docs, but it should be: > """Short desc. > > Longer desc if necessary. > > Args: > arg1: type Description of parameter > > Returns: > What return value means (and type). Done. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:95: return pfl On 2010/10/09 21:17:03, thmiller wrote: > Did you look at using googlecl.base.BaseCL.GetEntries() to do this? If you only > need exactly one portfolio, use GetSingleEntry(). I doubt it will be much better. Looks like I will end up reimplementing Portfolio query. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:102: """Create transaction.""" On 2010/10/09 21:17:03, thmiller wrote: > Documentation. (see above) Done. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:107: date = datetime.datetime.strptime(date, DATE_FORMAT) On 2010/10/09 21:17:03, thmiller wrote: > This can throw a ValueError if date does not match DATE_FORMAT. Done. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:141: # Portfolio On 2010/10/09 21:17:03, thmiller wrote: > What does this comment mean? Done. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:162: print fmt % ('Id', 'Title', 'Curr', 'Gain', 'Gain %', 'C.basis', On 2010/10/09 21:17:03, thmiller wrote: > You should let the user decide what fields to list with the --fields option. > picasa/service.py and contacts/base.py contain pretty extensive wrapper classes > to base yours off of, or you can choose another way to do it. I tried to use your wrappers, but failed because output concept is different for this service. I implemented my own formatters. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:162: print fmt % ('Id', 'Title', 'Curr', 'Gain', 'Gain %', 'C.basis', On 2010/10/09 21:17:03, thmiller wrote: > You should let the user decide what fields to list with the --fields option. > picasa/service.py and contacts/base.py contain pretty extensive wrapper classes > to base yours off of, or you can choose another way to do it. Done. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:232: client.DeleteTransaction(transaction) On 2010/10/09 21:17:03, thmiller wrote: > Take a look at DeleteTransaction. I bet we can use DeleteEntry instead, which is > sometimes helpful. I'm not sure I understand you here. Why using specific API should be avoided in favor of more low-level one? I think it's more readable and informative to use DeleteTransaction here. Moreover, if they change internals of DeleteTransaction we shouldn't do anything, but if we use DeleteEntry something might break in such a case. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:237: pfl = client.get_portfolio(options.title) On 2010/10/09 21:17:03, thmiller wrote: > Is a portfolio similar to a blog, where the user will most likely only have one > of them? Or is it more like an album in Picasa? Done. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:284: required = ['title', 'ticker', 'txnid']), On 2010/10/09 21:17:03, thmiller wrote: > Is there any shorthand for something like "list-transactions" that makes sense? > That's an awfully long task name. Done. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:292: # End: On 2010/10/09 21:17:03, thmiller wrote: > What are these comments for? I've never seen them, but they look potentially > useful. Done.
Sign in to reply to this message.
http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py File src/googlecl/finance/service.py (right): http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:95: return pfl On 2010/10/10 17:30:49, Ed Bartosh wrote: > On 2010/10/09 21:17:03, thmiller wrote: > > Did you look at using googlecl.base.BaseCL.GetEntries() to do this? If you > only > > need exactly one portfolio, use GetSingleEntry(). > I doubt it will be much better. Looks like I will end up reimplementing > Portfolio query. You should be able to use the PortfolioQuery, just call ToUri() on it before it gets sent to GetEntries. (see docs/service.py) GetEntries will do that title matching for you, support regular expression matches on the title, convert non-latin characters safely to the URL, etc. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:162: print fmt % ('Id', 'Title', 'Curr', 'Gain', 'Gain %', 'C.basis', On 2010/10/10 17:30:49, Ed Bartosh wrote: > On 2010/10/09 21:17:03, thmiller wrote: > > You should let the user decide what fields to list with the --fields option. > > picasa/service.py and contacts/base.py contain pretty extensive wrapper > classes > > to base yours off of, or you can choose another way to do it. > > Done. OK, the formatter looks pretty good. I may merge the two methods in the future. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:232: client.DeleteTransaction(transaction) On 2010/10/10 17:30:49, Ed Bartosh wrote: > On 2010/10/09 21:17:03, thmiller wrote: > > Take a look at DeleteTransaction. I bet we can use DeleteEntry instead, which > is > > sometimes helpful. > I'm not sure I understand you here. Why using specific API should be avoided in > favor of more low-level one? I think it's more readable and informative to use > DeleteTransaction here. Moreover, if they change internals of DeleteTransaction > we shouldn't do anything, but if we use DeleteEntry something might break in > such a case. All of the DeleteSpecificEntry functions in gdata are just wrappers to Delete. Actually, in Finance's case, poorly defined wrappers -- you can easily blunder into an AttributeError with DeletePortfolio. You're right that it's bad practice to make the assumption, but it's been that way since 1.2.4, and I don't expect that style to change any time soon. delete_entry_list will also prompt the user before deleting something, and allow a list of things to delete. As it's written now, the user has to call _run_delete_transaction for every transaction deleted. http://codereview.appspot.com/2412041/diff/8001/src/googlecl/finance/service.... src/googlecl/finance/service.py:342: This is an instance where it pays to use DeletePosition over googlecl's delete_entry_list, though that means that a "are you SURE?" prompt needs to be re-written for this case. Or delete_entry_list gets re-written with a callback, instead of using Delete() every time.
Sign in to reply to this message.
Hi Tom, Thank you for your suggestions. I reimplemented getting portolio using GetEntries and Getsingleentry and all delete* methods using DeleteEntryList as you suggested. Please review. Regards, Ed http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py File src/googlecl/finance/service.py (right): http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:95: return pfl On 2010/10/09 21:17:03, thmiller wrote: > Did you look at using googlecl.base.BaseCL.GetEntries() to do this? If you only > need exactly one portfolio, use GetSingleEntry(). Done. http://codereview.appspot.com/2412041/diff/1/src/googlecl/finance/service.py#... src/googlecl/finance/service.py:232: client.DeleteTransaction(transaction) On 2010/10/09 21:17:03, thmiller wrote: > Take a look at DeleteTransaction. I bet we can use DeleteEntry instead, which is > sometimes helpful. Done. http://codereview.appspot.com/2412041/diff/8001/src/googlecl/finance/service.... src/googlecl/finance/service.py:342: On 2010/10/13 21:59:44, thmiller wrote: > This is an instance where it pays to use DeletePosition over googlecl's > delete_entry_list, though that means that a "are you SURE?" prompt needs to be > re-written for this case. Or delete_entry_list gets re-written with a callback, > instead of using Delete() every time. Done.
Sign in to reply to this message.
Looking good! I'll incorporate this soon. The comment on base.py still stands, but that's separate from adding finance, so I'll make separate commits with changes to base.py that support deletion callbacks and no max-results. http://codereview.appspot.com/2412041/diff/13001/src/googlecl/base.py File src/googlecl/base.py (right): http://codereview.appspot.com/2412041/diff/13001/src/googlecl/base.py#newcode197 src/googlecl/base.py:197: if set_maxresults: If you really don't want to include a max-results parameter for Finance, could you set self.max_results = None in the constructor instead of adding a parameter to get_entries?
Sign in to reply to this message.
http://codereview.appspot.com/2412041/diff/13001/src/googlecl/base.py File src/googlecl/base.py (right): http://codereview.appspot.com/2412041/diff/13001/src/googlecl/base.py#newcode197 src/googlecl/base.py:197: if set_maxresults: On 2010/10/14 18:14:44, thmiller wrote: > If you really don't want to include a max-results parameter for Finance, could > you set self.max_results = None in the constructor instead of adding a parameter > to get_entries? I'd love to leave it like it was, but finance service doesn't support this parameter. This is what I got when trying to use GetEntries whithout modifications: "Failed to get entries: {'status': 403, 'body': 'This service does not support the 'max-results' parameter.', 'reason': 'Forbidden'}". I'm not sure that having attribute is better than having explicit parameter, but if you insist I'll do that.
Sign in to reply to this message.
http://codereview.appspot.com/2412041/diff/13001/src/googlecl/base.py File src/googlecl/base.py (right): http://codereview.appspot.com/2412041/diff/13001/src/googlecl/base.py#newcode197 src/googlecl/base.py:197: if set_maxresults: On 2010/10/14 18:14:44, thmiller wrote: > If you really don't want to include a max-results parameter for Finance, could > you set self.max_results = None in the constructor instead of adding a parameter > to get_entries? Done.
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
