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

changing the Coda tables can't make stampy completely crash #268

Open
MatthewBaggins opened this issue Jun 1, 2023 · 2 comments
Open

changing the Coda tables can't make stampy completely crash #268

MatthewBaggins opened this issue Jun 1, 2023 · 2 comments

Comments

@MatthewBaggins
Copy link
Collaborator

When loading tables from Coda, Stampy assumes they're going to contain columns with particular names.
There are 2 main ways this is problematic

  1. Hiding a column in Coda UI also hides it from API requests, so when Stampy tries to access that column, it causes a KeyError. This has already happened.
    Solution: Make Stampy download the entire table or, even better, some subset of its columns, regardless of what is visible in the UI
  2. Changing the name of a column causes the same kind of problem. This hasn't happen yet but might in the future.
    Solution: Make Stampy use column IDs which are immutable, so changing column names will not cause any problems

For reference, here is the library we use to interact with Coda: https://github.com/Blasterai/codaio
and here is Coda API documentation: https://coda.io/developers/apis/v1#section/Using-the-API

@MatthewBaggins
Copy link
Collaborator Author

I'm happy to do it, but can promise to have time for it no earlier than 19 VI.

@Aprillion
Copy link
Collaborator

Aprillion commented Jun 1, 2023

column ids can be deleted too, but I guess there is lower probability of unintentional damage (assuming accidental renaming is easier than deletion in Coda) + we already use table/view ids instead of table/view names => switching from column names to ids sounds OK

the main change to avoid Coda bugs could be to start using the raw Coda table with id grid-sync-1059-File with all columns instead of the Coda UI view with id table-YvPEyAXl8a that only shows the UI-visible columns from https://coda.io/d/_dfau7sl2hmG#_tutable-YvPEyAXl8a and is being used currently for

ALL_ANSWERS_TABLE_ID = "table-YvPEyAXl8a"

but the top priority should be make stampy not crash on Coda errors IMHO (or do we always want to crash on initialization Coda errors, just make those errors visible in #269? I think it shouldn't crash completely, but not a strong opinion)

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

No branches or pull requests

2 participants