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

Reset query status error code and error message after error #1094

Merged
merged 2 commits into from
Aug 16, 2022

Conversation

SimbaGithub
Copy link
Collaborator

Overview

SNOW-XXXXX

Description from #1005
enum QueryStatus has setErrorMessage and setErrorCode methods ref to put error message and error code.
Since Java's enum has single instance, if we change error message and error code these values will maintain permanently unless we call setErrorMessage and setErrorCode again.

External contributors - please answer these questions before submitting a pull request. Thanks!

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR adressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-592831: enum QueryStatus keeps previous errorCode and errorMessage #1005

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am modyfying authorization mechanisms
    • I am adding new credentials
    • I am modyfying OCSP code
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

    If the query status is not an error, set the error code and error message back to default values so they don't retain the error code/message.

Pre-review checklist

  • This change has passed precommit
  • I have reviewed code coverage report for my PR in (Sonarqube)

@SimbaGithub SimbaGithub force-pushed the resetQueryStatusErrorCodeAndMessageAfterError branch from 135add7 to 8047e13 Compare July 15, 2022 16:48
@SimbaGithub SimbaGithub marked this pull request as ready for review July 15, 2022 22:23
@SimbaGithub SimbaGithub force-pushed the resetQueryStatusErrorCodeAndMessageAfterError branch from 8047e13 to 5310645 Compare July 29, 2022 14:58
status.setErrorCode(2003);
status = rs1.unwrap(SnowflakeResultSet.class).getStatus();
// Assert status of query is a success
assertEquals(QueryStatus.SUCCESS, status);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good, except that the jenkins test server is a bit slow sometimes so this is coming back as RUNNING instead of SUCCESS. Putting in a brief thread sleep should fix this and prevent flaky tests. Something like a 3 second sleep timer.

Copy link
Contributor

@sfc-gh-mknister sfc-gh-mknister left a comment

Choose a reason for hiding this comment

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

Please add a sleep timer in the test to prevent a flaky test scenario where the async query hasn't finished running before it's asserted to be a success!

Other than that, looks good, so approving assuming that line will be added.

@sonarqubemergegate
Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@SimbaGithub SimbaGithub merged commit 9758960 into master Aug 16, 2022
@SimbaGithub SimbaGithub deleted the resetQueryStatusErrorCodeAndMessageAfterError branch August 16, 2022 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SNOW-592831: enum QueryStatus keeps previous errorCode and errorMessage
2 participants