search

duneanalytics / dune-client

A framework for interacting with Dune Analytics' officially supported API service
Apache License 2.0
85 stars 22 forks source link

Add upload_csv route #61

Closed bh2smith closed 1 year ago

bh2smith commented 1 year ago

This PR adds CSV upload functionality to the API client.

Unfortunately, it appears, I have the ability to completely overwrite other users tables... Not sure if this was intended. cc @msf

Tests are failing, because I assumed I would get a False success response when trying to overwrite someone else's table.

Closes #67

Remaining Question: Do we also was to update the async client? If so, it would probably need more work since it has not been kept updated like the other.

dsalv commented 1 year ago

This PR adds CSV upload functionality to the API client.

Unfortunately, it appears, I have the ability to completely overwrite other users tables... Not sure if this was intended. cc @msf

Tests are failing, because I assumed I would get a False success response when trying to overwrite someone else's table.

Closes #67

Remaining Question: Do we also was to update the async client? If so, it would probably need more work since it has not been kept updated like the other.

The ability to overwrite other people's tables under dune_upload is the intended behavior. Tables cannot be overwritten if they are under users' personal namespaces (e.g. dune.bh2smith.dataset_{table_name}). We introduced personal namespaces last week. I am not sure that would be helpful in your tests

bh2smith commented 1 year ago

So far, it looks like just commentary on intended functionality. At the moment the implementation and tests are aligned with the intended functionality. If its not blocking, I'd think we can merge as is and adjust later as necessary.

Please let me know if there is anything that should be changed here and feel free to approve if all is good.