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

Test datetime attribute insert and match time-zone invariant #424

Merged
merged 9 commits into from
Jul 3, 2023
Merged

Test datetime attribute insert and match time-zone invariant #424

merged 9 commits into from
Jul 3, 2023

Conversation

shiladitya-mukherjee
Copy link
Contributor

@shiladitya-mukherjee shiladitya-mukherjee commented Jun 23, 2023 •
edited
Loading

What is the goal of this PR?

Added step implementation for changing environment timezone for Java, and fixed the parsing of datetime. We hence enabled the BDD tests to check timezone-invariance of inserting and reading datetime attributes.

What are the changes implemented in this PR?

  • Added "set time-zone is: {time_zone_label}" which sets environment timezone to time_zone_label.
  • Fixed datetime attribute parsing by bounding the number of splits of the attribute by ":" to 2.

@typedb-bot
Copy link
Member

typedb-bot commented Jun 23, 2023 •
edited by flyingsilverfin
Loading

PR Review Checklist

Do not edit the content of this comment. The PR reviewer should simply update this comment by ticking each review item below, as they get completed.

Trivial Change

  • This change is trivial and does not require a code or architecture review.

Code

  • Packages, classes, and methods have a single domain of responsibility.
  • Packages, classes, and methods are grouped into cohesive and consistent domain model.
  • The code is canonical and the minimum required to achieve the goal.
  • Modules, libraries, and APIs are easy to use, robust (foolproof and not errorprone), and tested.
  • Logic and naming has clear narrative that communicates the accurate intent and responsibility of each module (e.g. method, class, etc.).
  • The code is algorithmically efficient and scalable for the whole application.

Architecture

  • Any required refactoring is completed, and the architecture does not introduce technical debt incidentally.
  • Any required build and release automations are updated and/or implemented.
  • Any new components follows a consistent style with respect to the pre-existing codebase.
  • The architecture intuitively reflects the application domain, and is easy to understand.
  • The architecture has a well-defined hierarchy of encapsulated components.
  • The architecture is extensible and scalable.

krishnangovindraj
krishnangovindraj requested changes Jun 26, 2023
View reviewed changes
Copy link
Member

@krishnangovindraj krishnangovindraj left a comment

Choose a reason for hiding this comment

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

Mostly styling.

test/behaviour/concept/serialization/json/BUILD Outdated Show resolved Hide resolved
test/behaviour/concept/thing/attribute/AttributeSteps.java Outdated Show resolved Hide resolved
test/behaviour/typeql/TypeQLSteps.java Outdated Show resolved Hide resolved
test/behaviour/util/UtilSteps.java Show resolved Hide resolved
@flyingsilverfin flyingsilverfin changed the title Step implementation for changing environment timezone. Step implementation for changing environment timezone Jun 27, 2023
flyingsilverfin
flyingsilverfin reviewed Jun 27, 2023
View reviewed changes
test/behaviour/typeql/TypeQLSteps.java
@@ -645,13 +644,13 @@ public boolean check(Concept concept) {
}
}

public static class AttributeUniquenessCheck {
public static abstract class AttributeUniquenessCheck {
Copy link
Member

Choose a reason for hiding this comment

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

nice catch!

flyingsilverfin
flyingsilverfin reviewed Jun 27, 2023
View reviewed changes
test/behaviour/typeql/TypeQLSteps.java

protected final Label type;
protected final String value;

AttributeUniquenessCheck(String typeAndValue) {
String[] s = typeAndValue.split(":");
String[] s = typeAndValue.split(":", 2);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a huge fan of this but I think we'll take it for now. Thoughts: it's not obvious how this is going to interplace with the next assertion line, and what it means about the format of the 'typeAndValue' identifier we're expecting (less explainable in 1 short sentence IMO)

@shiladitya-mukherjee shiladitya-mukherjee added this to the Bug Fixes milestone Jun 27, 2023
@shiladitya-mukherjee shiladitya-mukherjee changed the title Step implementation for changing environment timezone Made datetime attribute insert and match time-zone invariant Jun 29, 2023
krishnangovindraj
krishnangovindraj requested changes Jun 29, 2023
View reviewed changes
Copy link
Member

@krishnangovindraj krishnangovindraj left a comment

Choose a reason for hiding this comment

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

Please check how we set the timezone

@shiladitya-mukherjee shiladitya-mukherjee changed the title Made datetime attribute insert and match time-zone invariant Test datetime attribute insert and match time-zone invariant Jun 30, 2023
flyingsilverfin
flyingsilverfin approved these changes Jul 3, 2023
View reviewed changes
Copy link
Member

@flyingsilverfin flyingsilverfin left a comment

Choose a reason for hiding this comment

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

Good!

@flyingsilverfin flyingsilverfin merged commit 0e13f97 into typedb:master Jul 3, 2023
dmitrii-ubskii pushed a commit to dmitrii-ubskii/typedb-driver that referenced this pull request Jul 14, 2023
@shiladitya-mukherjee @dmitrii-ubskii
cherry-pick: Test datetime attribute insert and match time-zone invar…
b82ba0e 
…iant (typedb#424)

Added step implementation for changing environment timezone for Java, and fixed the parsing of datetime. We hence enabled the BDD tests to check timezone-invariance of inserting and reading datetime attributes.

- Added "set time-zone is: {time_zone_label}" which sets environment timezone to time_zone_label.
- Fixed datetime attribute parsing by bounding the number of splits of the attribute by ":" to 2.
dmitrii-ubskii pushed a commit to dmitrii-ubskii/typedb-driver that referenced this pull request Jul 14, 2023
@shiladitya-mukherjee @dmitrii-ubskii
cherry-pick: Test datetime attribute insert and match time-zone invar…
66a34cd 
…iant (typedb#424)

Added step implementation for changing environment timezone for Java, and fixed the parsing of datetime. We hence enabled the BDD tests to check timezone-invariance of inserting and reading datetime attributes.

- Added "set time-zone is: {time_zone_label}" which sets environment timezone to time_zone_label.
- Fixed datetime attribute parsing by bounding the number of splits of the attribute by ":" to 2.
@dmitrii-ubskii dmitrii-ubskii mentioned this pull request Aug 9, 2023
Merged
dmitrii-ubskii added a commit that referenced this pull request Aug 22, 2023
@dmitrii-ubskii @benjaminasmall
@flyingsilverfin @krishnangovindraj
Implement expressions and top-level values in client-rust (#434)
5fdec21 
## What is the goal of this PR?

We update the Rust driver to support the features introduced in driver
Java since v2.19 in effort to preserve the feature set when
transitioning from the JVM-native implementation to the Rust JNI
implementation.

## What are the changes implemented in this PR?

Changes in master since branching (#417):

Reimplemented in Rust and made available in Java over JNI:
- #409 
- #421 
- 1f396a6 Improve method unavailable error message
- #430 
- #431: partial, since `tonic` does not report SSL errors to the same
granularity

Cherry-picked directly:
- #415
- 42800e7 Update VERSION to 2.18.1
- 7cd0a5a Add eclipsesource-minimal-json to maven dependencies (#423)
- 63614ec Update CODEOWNERS
- #424

Dropped entirely (Java-specific or obsolete):
- #422 
- 8c0caa1 Update VERSION and release notes

---------

Co-authored-by: Benjamin Small <benjaminasmall@gmail.com>
Co-authored-by: joshua <joshua@vaticle.com>
Co-authored-by: Krishnan Govindraj <krishnangovindraj@users.noreply.github.com>
@flyingsilverfin flyingsilverfin modified the milestones: Bug Fixes, 2.24.0 Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krishnangovindraj krishnangovindraj krishnangovindraj approved these changes

@flyingsilverfin flyingsilverfin flyingsilverfin approved these changes

@alexjpwalker alexjpwalker Awaiting requested review from alexjpwalker

@haikalpribadi haikalpribadi Awaiting requested review from haikalpribadi

@benjaminasmall benjaminasmall Awaiting requested review from benjaminasmall

Assignees

@krishnangovindraj krishnangovindraj

@alexjpwalker alexjpwalker

@benjaminasmall benjaminasmall

@shiladitya-mukherjee shiladitya-mukherjee

Labels
priority: high type: bug
Projects
None yet
Milestone
2.24.0
Development

Successfully merging this pull request may close these issues.
7 participants
@shiladitya-mukherjee @typedb-bot @flyingsilverfin @krishnangovindraj @haikalpribadi @alexjpwalker @benjaminasmall