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.

Sign up for GitHub

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

Jump to bottom

Collections Datastore #2620

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Collections Datastore #2620

wants to merge 8 commits into from

Conversation

stuartc
Copy link
Member

@stuartc stuartc commented Oct 30, 2024 •
edited by josephjclark
Loading

Description

This PR [adds/changes/fixes]... (A description of your work goes here.)

Re: #2190

Spec: #2566

Validation steps

  1. (How can a reviewer validate your work?)

Additional notes for the reviewer

  1. (Is there anything else the reviewer should know or look out for?)

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed a self-review of my code.
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

jyeshe and others added 8 commits October 22, 2024 11:14
@jyeshe
Add workflow collections (#2569)
fd14f8f 
* Add workflow collections

These are named and shared key-value storages

* Increase timestamp precision

* Use single return patter on same get function

* Optimize all ops once the names are stable

There is no user update on collection name

* Changelog and formatting

* Add stream_all allowing equivalent queries to HGETALL and HSCAN

* Add stream_match for wildcard prefix queries

* Add pg_trigram and GIN index on the key to allow multi wildcard
@jyeshe @stuartc
Sort by ascending upsert timestamp
30e08b5
@jyeshe @stuartc
Use only unique constraint (required and non-relational to save a BTREE)
6b22d9a 
Also changes timestamp type
@jyeshe
Fix collections cursor and add benchmarking (#2604)
9c7cf60
@jyeshe
Enable put_all on Collections (#2609)
31775b0
@stuartc
Merge remote-tracking branch 'origin/main' into feature/collections
98f23b4 
[ci skip]
@elias-ba
Collections CRUD UI (#2587)
6edc03e 
* Collections CRUD UI

* Mix verify

* Code readability issue

* Render raw_name errors

* Sorting by name

* Tests for copy_error/4

* Tests for url_safe_name/1

* Tests for list_collections/1, create_collection/1, and update_collection/2

* Format aliases

* Tests for collection live

* Test cancel collection creation modal

* Collection tests

* Update CHANGELOG
@stuartc @jyeshe
Collections API (#2603)
8255a5e 
* streaming collections api WIP

* Refactoring UserTokens

- Add sub claim to UserToken
- Abstract RSA key generation

This change reduces the requirement to shell out to `openssl` to
generate RSA key pairs, in addition the functions have been moved into
a common `Utils.Crypto` module so the functions can be used elsewhere.

- Add common token_signer for new api tokens
- Move UserToken for API auth to Tokens.PersonalAccessToken
- Workers.Token -> Workers.WorkerToken

* Conform RunToken generation to use RunOptions

* Authentication and authorization for Collections API

* Streaming response for stream_all

- Added `has_many :items` to Collection model allowing easy creation of
  items during tests.
- Added Jason.Encoder protocol to Collections.Item for serializing.
- Collections controller sends through a streaming response for "all".
- Conform FallbackControlly and templates to all (and optionally)
  support `@error`

**NOTE** The controller has a few things to note:

- It does not conform to the entire spec, as only one route so far.
- The shape of the response is currently _only_ an array and not an
  object with `values` and `metadata`.
- There is some cleanup and refactoring required with the authentication
  etc, for now there is a `action/2` callback where all the resulting
  plugs and pipelines will be built from.

* Collections API with cursor (#2616)

* Implements collections REST api (including streaming)

commit 810228e
Author: Rogerio Pontual <[email protected]>
Date:   Tue Oct 29 17:07:47 2024 +0100

    Fix POST to work with multiple {key: key, value: value}

commit 4321f4f
Author: Rogerio Pontual <[email protected]>
Date:   Tue Oct 29 15:55:45 2024 +0100

    Implements remaining http verbs

    - post, put, get and delete

commit de1923a
Author: Rogerio Pontual <[email protected]>
Date:   Tue Oct 29 08:25:20 2024 +0100

    Streaming to network (local) while streaming from database

    - Additionally uses cursor and limit

commit 2bc0d94
Author: Stuart Corbishley <[email protected]>
Date:   Sun Oct 27 13:16:33 2024 +0200

    Streaming response for stream_all

    - Added `has_many :items` to Collection model allowing easy creation of
      items during tests.
    - Added Jason.Encoder protocol to Collections.Item for serializing.
    - Collections controller sends through a streaming response for "all".
    - Conform FallbackControlly and templates to all (and optionally)
      support `@error`

    **NOTE** The controller has a few things to note:

    - It does not conform to the entire spec, as only one route so far.
    - The shape of the response is currently _only_ an array and not an
      object with `values` and `metadata`.
    - There is some cleanup and refactoring required with the authentication
      etc, for now there is a `action/2` callback where all the resulting
      plugs and pipelines will be built from.

commit cab0565
Author: Stuart Corbishley <[email protected]>
Date:   Thu Oct 24 12:00:29 2024 +0200

    Authentication and authorization for Collections API

commit 3d7f511
Author: Stuart Corbishley <[email protected]>
Date:   Tue Oct 22 14:28:32 2024 +0200

    Conform RunToken generation to use RunOptions

commit 9eae9e6
Author: Stuart Corbishley <[email protected]>
Date:   Tue Oct 15 13:01:19 2024 +0200

    Refactoring UserTokens

    - Add sub claim to UserToken
    - Abstract RSA key generation

    This change reduces the requirement to shell out to `openssl` to
    generate RSA key pairs, in addition the functions have been moved into
    a common `Utils.Crypto` module so the functions can be used elsewhere.

    - Add common token_signer for new api tokens
    - Move UserToken for API auth to Tokens.PersonalAccessToken
    - Workers.Token -> Workers.WorkerToken

commit 03e6a32
Author: Stuart Corbishley <[email protected]>
Date:   Wed Oct 9 08:26:59 2024 +0200

    streaming collections api WIP

commit 6edc03e
Author: Elias W. BA <[email protected]>
Date:   Sun Oct 27 12:17:08 2024 +0000

    Collections CRUD UI (#2587)

    * Collections CRUD UI

    * Mix verify

    * Code readability issue

    * Render raw_name errors

    * Sorting by name

    * Tests for copy_error/4

    * Tests for url_safe_name/1

    * Tests for list_collections/1, create_collection/1, and update_collection/2

    * Format aliases

    * Tests for collection live

    * Test cancel collection creation modal

    * Collection tests

    * Update CHANGELOG

* Add records with created_at timestamps and unique updated_at

* Formatting responses

---------

Co-authored-by: Rogerio Pontual <[email protected]>
@josephjclark
Copy link
Contributor

josephjclark commented Oct 30, 2024 •
edited
Loading

So far as I understand it the following still needs doing:

  • POST should return 200/201
  • DELETE does not support key patterns
  • Support for created_at etc query parameters on GET and DELETE
  • Support for created_at metadata in the database
  • Rename upserts to upserted ?

Any of these that we don't plan to do before release need to be spun out into new issues

And I'm seeing these issues on my local branch:

  • GET with a key returns { key, value }, not { items: [{ key, value }] }
  • Key patterns seem to not work on GET (see next comment)

@josephjclark
Copy link
Contributor

Yes Ok, to confirm some issues:

curl -X GET localhost:4000/collections/stuff/a incorrectly returns { value, key } (it should be in an items array for consistency

curl -X GET localhost:4000/collections/stuff/* incorrectly returns { error: "item not found" }. But I expect a pattern search here (this is basically get all)

curl -X GET localhost:4000/collections/stuff/ correctly returns { items, cursor }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
v2
Status: New Issues
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stuartc @josephjclark @jyeshe @elias-ba