Skip to content
This repository was archived by the owner on Oct 9, 2023. It is now read-only.

Conversation

vmax
Copy link
Contributor

@vmax vmax commented Aug 14, 2019

What is the goal of this PR?

Recent changes in protocol (typedb/typedb-protocol#7) allow keyspace operations to be authenticated. This PR adapts Grakn Client NodeJS to these recent changes.

What are the changes implemented in this PR?

  • KeyspaceService.retrieve and KeyspaceService.delete now properly set credentials
  • Bump @graknlabs_protocol and @graknlabs_grakn_core to latest version

@vmax vmax added this to the 1.5.4 milestone Aug 14, 2019
@vmax vmax requested a review from marco-scoppetta August 14, 2019 13:47
}
if (this.credentials.password !== undefined) {
retrieveRequest.setPassword(this.credentials.password);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can just:

if(this.credentials){
    retrieveRequest.setUsername(this.credentials.username);
    retrieveRequest.setPassword(this.credentials.password);
}

Your code should throw exception if this.credentials is null, have you run the tests already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marco-scoppetta addressed in 2ebb12b

I currently have a problem running tests locally (even on non-modified master) so I relied on CI here. Good catch!

Copy link
Contributor

Choose a reason for hiding this comment

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

Strange, I don't see the tests being run on CI either

@vmax vmax merged commit 65e537a into typedb:master Aug 14, 2019
@vmax vmax deleted the authenticated-keyspace-operations branch August 14, 2019 14:02
dmitrii-ubskii pushed a commit to dmitrii-ubskii/typedb-client-nodejs that referenced this pull request Sep 1, 2023
## What is the goal of this PR?

Adapt `@graknlabs_client_java` to latest changes in bazel-distribution (in particular, typedb/bazel-distribution#150)

## What are the changes implemented in this PR?

Instead of supplying `version_file` everywhere, use `--define version=<>` as a Bazel argument to supply version.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants