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

Break Grakn-Core Dependency #39

Merged
merged 22 commits into from
Oct 1, 2019

Conversation

flyingsilverfin
Copy link
Member

@flyingsilverfin flyingsilverfin commented Sep 27, 2019

What is the goal of this PR?

Remove code dependencies of Client-Java on Grakn Core, specifically api.Transaction and grakn.core.concept interfaces. This will allow us to update Client-Java independently of Grakn, only fixing compatibility issues mandated by the protocol, in line with the other drivers.

Resolves #5288, #5289, and part of #5272

What are the changes implemented in this PR?

  • Remove api.Transaction and grakn.core.concept interface depenendencies in Client-Java
  • Move ConceptIT into behavior, TODO refactor into BDD framework
  • rename all RemoteConcept.java classes to just follow Concept.java pattern, removing the Remote naming scheme
  • Migrate and modify the required missing classes from Grakn-Core (copy-paste, then remove unnecessary methods and interfaces)
  • Create Answer hierarchy that was missing before.

case BOOLEAN:
return AttributeType.DataType.BOOLEAN;
return grakn.client.concept.AttributeType.DataType.BOOLEAN;
Copy link
Member

Choose a reason for hiding this comment

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

these package addresses can be collapsed, @flyingsilverfin. It's okay to do it in the next PR.

Copy link
Member

@haikalpribadi haikalpribadi left a comment

Choose a reason for hiding this comment

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

awesome 👍

@haikalpribadi haikalpribadi added this to the 1.5.6 milestone Oct 1, 2019
@haikalpribadi haikalpribadi merged commit c909167 into typedb:master Oct 1, 2019
flyingsilverfin added a commit that referenced this pull request Oct 1, 2019
## What is the goal of this PR?
Previous PR #39 neglected to implement `AutoClosable` on `Session`, which is expected in Grakn Core for now

## What are the changes implemented in this PR?
* Implement `AutoClosable`
@flyingsilverfin flyingsilverfin deleted the break-core-dependency branch October 2, 2019 12:06
dmitrii-ubskii added a commit to dmitrii-ubskii/typedb-driver that referenced this pull request Jul 13, 2023
## What is the goal of this PR?

We improved parse errors handling in the `behaviour` test package.

## What are the changes implemented in this PR?

We added asserts at the all stages of query processing for "`typeql
define [...]`" and "`typeql insert [...]`" expressions.

---------

Co-authored-by: Dmitrii Ubskii <[email protected]>
dmitrii-ubskii pushed a commit that referenced this pull request Aug 25, 2023
…ac-zip (#39)

## What is the goal of this PR?

Test should unpack, start/stop Grakn distribution _within_ the test

## What are the changes implemented in this PR?

* tests now depend on `@graknlabs_grakn_core//:assemble-mac-zip` via `data`
* tests now unpack/start/stop Grakn before executing
dmitrii-ubskii pushed a commit to dmitrii-ubskii/typedb-driver that referenced this pull request Aug 31, 2023
…5.2 (typedb#39)

## What is the goal of this PR?

Grakn Core 1.5.2 introduced a protocol change which rendered `client-nodejs` version 1.5.1 and older incompatible. We have updated `client-nodejs` to allow it to connect to Grakn Core and KGMS 1.5.2.

The changes that had been introduced are internal changes which did not affect the public API.

## What are the changes implemented in this PR?

- Internally, `username` and `password` are now supplied when making the `Session.Open.Req` gRPC request as opposed to 1.5.1 where they were supplied when making the `Transaction.Open.Req` request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants