gspread icon indicating copy to clipboard operation
gspread copied to clipboard

Update local properties when updating spreadsheet/worksheet

Open lavigne958 opened this issue 3 years ago • 2 comments

Is your feature request related to a problem? Please describe. Updating spreadsheet property call does not update local properties.

ex: call to append_row does not increase property row_count

Describe the solution you'd like Update local properties when necessary.

Describe alternatives you've considered One can make the explicit call to sht.fetch_sheet_metadata() to update the properties with latest known values from the API.

Additional context Warning this only solves the issue for SpreadSheets accessed via a single script. If an other script or a human using a web browser accesses the SpreadSheet and updates it, then the values known localy are still invalid.

If you are in that case then you must call sht.fetch_sheet_metadata() to prevent concurent accesses etc.

relates to #545

lavigne958 avatar May 24 '21 09:05 lavigne958

Issue

This issue raises the following problems:

  • update the spreadsheet (and underlying worksheet) is not reflected locally
  • not having the latests state of the spreadsheet locally brings some unexpected behavior
    • updating the row or column count brings column/row insertion to the wrong place
    • updating the title can break any function that tries to get a value using a range (because ranges are build using sheet title)
  • the spreadsheet can be updated online using the UI and the running script can't be notified about the changes.

Requirements

To solve we must ensure the following requirements:

  • The use of gspread can be limited by the number of API calls, this should not be raised extensively otherwise users will be blocked and won't be able to send any more requests for some time.
  • The local spreadsheet properties should reflect what has been update online.

Possible solutions

update local properties after successful update call

  • for any method that updates the spreadsheet properties:

  • update the local properties

  • solves the basics cases and bug pointed by issues linked to this issue.

  • does not solves concurrent access to the spreadsheet (multiple scripts accessing the same spreadsheet)

  • after an update, getting the same spreadsheet can result in different properties (so different behaviors) if spreadhseet has been updated in between

fetch latest spreadsheet properties after every update call

  • for any method that updates the spreadsheet properties

    • fetch the spreadsheet properties
  • solves the basics cases and cases when someone else update the spreadsheet online (bypasses the running script).

  • increases the number of API calls by a lot.

  • This is not suitable for production for heavy usage of google API

Proposal

Update local properties and provide convenient method to fetch properties

  • When a method updates a properties online, on successful call update local property too
    • append a new column will increase the column count, etc...
  • Provide a new method that will fetch the latest spreadsheet properties and sheet properties.
    • can be called by the users or the library itself at any time, this way the user is in control of the extra API call the library makes.
  • this solution does not extra API calls, it update the local properties based on what the script is sending to the google API and offers a way to fetch the properties.

lavigne958 avatar Oct 04 '21 11:10 lavigne958

Possible solutions

Use some caching mechanism

  • In some scenarios the user knows it is the only one accessing the spreadsheet

    • no concurrent accesses
  • Some caching mechanism can be activated at the constructor of the spreadsheet

  • it will fetch most of the spreadsheet properties at init

  • it can (only ?) mostly rely on every interactions it makes and assume it is the only one editing the resource

  • In case the first assumption fails the whole thing brakes, changes will be made with possible out of date data

  • This is risky and could break other use of gspread, if the cache is not activated and a get_* method does not make the HTTP call it becomes a critical issue.

Thank to @sandl99 for this proposal

lavigne958 avatar Dec 09 '21 09:12 lavigne958

"Fixed" by #1211?

Also the same (but more info here) proposal as #1212

(close one of the this or #1212 as duplicate? not sure which)

alifeee avatar Jun 08 '23 19:06 alifeee

I agree that #1211 fixes most of them. Did we go over the whole properties we set ? if so yes we can close this issue then.

I think #1212 will be too costly for users.

We can add a new feature for the user to choose either:

  • get properties from API each time it requests a property
  • get the local known property to reduce API calls.

lavigne958 avatar Jun 14 '23 13:06 lavigne958

See https://github.com/burnash/gspread/pull/1211#issuecomment-1591326346 for every property that is set.

alifeee avatar Jun 14 '23 14:06 alifeee

calls to self.spreadsheet.values_append do not update the properties row_count and col_count, which is the original issue in this issue.

Perhaps there are other methods which change @propertys of gspread.Worksheet? I am not sure.

Here, we would have to update properties row_count and col_count on calls to append_rows, insert_rows, insert_cols, delete_dimension, and perhaps other methods. Since the solution here seems to be "try remember to update the local properties", I am not sure of what a robust solution would be.

alifeee avatar Jun 14 '23 14:06 alifeee

perhaps we could have some kind of wrapper around return self.spreadsheet.batch_update(body) so that the developer is 'forced'/coerced into updating local state when updating props

i.e., rework this signature

https://github.com/burnash/gspread/blob/c9c88a7f7366f060293f6c1b26008e52166314ad/gspread/worksheet.py#L2790-L2792

alifeee avatar Jun 14 '23 14:06 alifeee

I thought about it too but too complexe I think :thinking: (the wrapper around batch_update)

I check the code today and when we use append_row we use the internal method resize that increase the row count right ?

lavigne958 avatar Jun 14 '23 14:06 lavigne958

gspread.Worksheet.add_rows uses gspread.Worksheet.resize, which does update Worksheet.row_count.

However,

gspread.Worksheet.append_rows uses gspread.Spreadsheet.values_append, which doesn't update Worksheet.row_count.

I am not very familiar with the spreadsheet object, so I am not sure why the implementation differs in this way.

alifeee avatar Jun 14 '23 14:06 alifeee

this is because they don't use the same API calls.

I see so we miss some of them :thinking:

We are still better than before but not yet complete.

We could simply increase the value in the method append_rows ?? that should work right ?

lavigne958 avatar Jun 15 '23 18:06 lavigne958

this is now fixed

alifeee avatar Jun 29 '23 13:06 alifeee