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

Follow up for PR #1251: Remove Optional and centralize validation #1259

Merged
merged 2 commits into from
Jul 10, 2024
Merged

Follow up for PR #1251: Remove Optional and centralize validation #1259

merged 2 commits into from
Jul 10, 2024

Conversation

Hazel-Datastax
Copy link
Contributor

@Hazel-Datastax Hazel-Datastax commented Jul 10, 2024 •
edited
Loading

What this PR does:
Follow up for PR #1251: Remove Optional and centralize validation

Which issue(s) this PR fixes:
Fixes NA

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CLA Signed: DataStax CLA

@Hazel-Datastax Hazel-Datastax requested a review from a team as a code owner July 10, 2024 21:11
tatu-at-datastax
tatu-at-datastax reviewed Jul 10, 2024
View reviewed changes
...ava/io/stargate/sgv2/jsonapi/service/embedding/operation/test/CustomITEmbeddingProvider.java Outdated
List<float[]> response = new ArrayList<>(texts.size());
if (texts.size() == 0) return Uni.createFrom().item(Response.of(batchId, response));
if (!apiKey.isPresent() || !apiKey.get().equals(TEST_API_KEY))
if (null == apiKey || !apiKey.equals(TEST_API_KEY))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine as is, but can simplify as:

if (!TEST_API_KEY.equals(apiKey)) {

since String.equals(null) is valid (returns false)

tatu-at-datastax
tatu-at-datastax approved these changes Jul 10, 2024
View reviewed changes
Copy link
Contributor

@tatu-at-datastax tatu-at-datastax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Hazel-Datastax Hazel-Datastax merged commit 018680b into main Jul 10, 2024
4 checks passed
@Hazel-Datastax Hazel-Datastax deleted the hazel/code_refactor branch July 10, 2024 22:06
@Hazel-Datastax Hazel-Datastax mentioned this pull request Jul 11, 2024
Merged
vkarpov15
vkarpov15 reviewed Jul 11, 2024
View reviewed changes
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

amorton added a commit that referenced this pull request Jul 16, 2024
@amorton
Table Refactoring - working insert and find
e29a796 
This commit gets the insert and find working in limited cases.

Find does not support any clauses, just returns everything. Insert
expects the document to have a field called `key`

The feature flag to enable tables has been renamed to fit convention, it
is now `-Dstargate.tables.enabled=true`

Full command to start quarkus in dev against HCD container would be:

./mvnw quarkus:dev -Dstargate.data-store.ignore-bridge=true \
-Dstargate.jsonapi.operations.vectorize-enabled=true \
-Dstargate.jsonapi.operations.database-config.local-datacenter=dc1 \
-Dquarkus.log.console.darken=2 -Dstargate.tables.enabled=true -Poffline

This is the 3rd chunk of changes, it is built against the work in
ajm/tables-chunk-2 branch.

it pulls commits from the ajm/tables branch listed below:

commit e35c78b
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Thu Jul 11 13:40:20 2024 +1200

    fmt fix

commit 64f2993
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Thu Jul 11 13:37:20 2024 +1200

    fmt fix

-> Skipped - merge commit <-
commit d69d2c6
Merge: f7bad22 018680b
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Thu Jul 11 13:31:19 2024 +1200

    Merge branch 'main' into ajm/tables

commit f7bad22
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Thu Jul 11 11:52:49 2024 +1200

    Test fixes for Integration tests

    ReadDocument now accepts an optional docID

    see comments in ReadAndUpdateOperation about how we create an
    upsert doc

commit adb2fa9
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Thu Jul 11 10:40:14 2024 +1200

    clean for InsertAttempt

    comments up to standard, removed unused getRow()

commit 4985a94
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Wed Jul 10 15:24:26 2024 -0700

    Replace FeatureFlags with Quarkus config setting (#1257)

-> skipped - already in main <-
commit 018680b
Author: Hazel <hazel.he@datastax.com>
Date:   Wed Jul 10 18:06:48 2024 -0400

    Follow up for PR #1251: Remove `Optional` and centralize validation (#1259)

-> SKIPPED - NOT IN MAIN BUT SAME AS 9d977cd below <-
commit bd6be18
Author: Hazel <hazel.he@datastax.com>
Date:   Wed Jul 10 16:41:24 2024 -0400

    Improve error message when EGW timeout (#1255)

-> SKIPPED - NOT IN MAIN BUT SAME AS 197bfd2 below <-
commit 33311d2
Author: Hazel <hazel.he@datastax.com>
Date:   Wed Jul 10 15:12:45 2024 -0400

    Small fix: Improve error message when not providing provider key through `x-embedding-api-key` (#1251)

-> SKIPPED - NOT IN MAIN BUT SAME AS dd652c3 below <-
commit 486977f
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Jul 10 11:44:53 2024 -0400

    Use Stargate v2.1.0-BETA-13 (#1252)

    Co-authored-by: jeffreyscarpenter <12115970+jeffreyscarpenter@users.noreply.github.com>

commit 611282a
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Thu Jul 11 10:00:03 2024 +1200

    Insert working, one and many - some Test failing

-> skipped - already in main <-
commit 9d977cd
Author: Hazel <hazel.he@datastax.com>
Date:   Wed Jul 10 16:41:24 2024 -0400

    Improve error message when EGW timeout (#1255)

-> skipped - already in main <-
commit 197bfd2
Author: Hazel <hazel.he@datastax.com>
Date:   Wed Jul 10 15:12:45 2024 -0400

    Small fix: Improve error message when not providing provider key through `x-embedding-api-key` (#1251)

-> skipped - already in main <-
commit dd652c3
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Jul 10 11:44:53 2024 -0400

    Use Stargate v2.1.0-BETA-13 (#1252)

    Co-authored-by: jeffreyscarpenter <12115970+jeffreyscarpenter@users.noreply.github.com>

commit 7e6e99d
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Wed Jul 10 16:37:47 2024 +1200

    refactor Insert results ready for tables

commit 588b297
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Wed Jul 10 13:47:06 2024 +1200

    Refactor ReadOperationPage for reuse with tables

    and added DocumentSource

commit d62f95c
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Wed Jul 10 10:34:56 2024 +1200

    Basic findOne and find for tables

    just does a select *, in place to find re-use with collection
    commands

commit fbd09e9
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Tue Jul 9 18:08:16 2024 -0700

    Enable metadata access to "system" keyspace (for testing purposes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vkarpov15 vkarpov15 vkarpov15 left review comments

@tatu-at-datastax tatu-at-datastax tatu-at-datastax approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Hazel-Datastax @vkarpov15 @tatu-at-datastax