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

feat(csharp/src/Drivers/Apache/Spark): implement async overrides for Spark driver #1830

Merged

Conversation

birschick-bq
Copy link
Contributor

Implements asynchronous overrides for the Spark driver.

  • Implements ExecuteUpdateAsync
  • Implements ExecuteQueryAsync
  • Refactor methods and names to use ...Async, where appropriate.
  • Refactor tests base to use async methods

@birschick-bq birschick-bq marked this pull request as ready for review May 8, 2024 16:35
csharp/src/Drivers/Apache/Hive2/HiveServer2Connection.cs Outdated Show resolved Hide resolved
{
TGetOperationStatusResp? statusResponse = null;
do
{
if (statusResponse != null) { Thread.Sleep(500); }
if (statusResponse != null) { await Task.Delay(500); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Not blocking, but it would be nice if this were configurable via a statement option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, created class properties to hold the value. Will add an implementation of the AdbcStatement.SetOption in a future PR.

public override QueryResult ExecuteQuery() => ExecuteQueryAsync().AsTask().Result;

public override UpdateResult ExecuteUpdate() => ExecuteUpdateAsync().Result;

Copy link
Contributor

Choose a reason for hiding this comment

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

Should these implementations (and the one in ImpalaStatement) move into the base class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move the implementations of ExecuteQuery, ExecuteUpdate, ExecuteQueryAsync and ExecuteUpdateAsync to the base class.

}
*/
}

public override IArrowArrayStream GetInfo(IReadOnlyList<AdbcInfoCode> codes)
Copy link
Contributor

Choose a reason for hiding this comment

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

We are going to need to close the session on the Spark side or the resources will remain in use. Please file a followup bug for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to be clear, if you're referring to the removed Dispose, there is a correct implementation of the Dispose in the base class HiveServer2Statement.

@CurtHagenlocher CurtHagenlocher merged commit 4d08122 into apache:main May 9, 2024
6 checks passed
@lidavidm lidavidm added this to the ADBC Libraries 13 milestone May 9, 2024
@birschick-bq birschick-bq deleted the dev/birschick-bq/execute-async branch May 9, 2024 19:05
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.

3 participants