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

Implement Stream Transaction Api. #323

Merged
merged 4 commits into from
Jul 4, 2021

Conversation

DiscJockeyDJ
Copy link
Contributor

Implemented the Stream Transaction Api using the documentation from Arangodb

https://www.arangodb.com/docs/3.6/http/transaction-stream-transaction.html

Even though the API calls work, the collections gets updated once the transactions begins (or) before committing/aborting.

Added few test cases to check that.

@DiscJockeyDJ DiscJockeyDJ force-pushed the streamTransaction branch 2 times, most recently from 5264cfd to d4afd34 Compare June 4, 2021 15:08
@DiscoPYF DiscoPYF linked an issue Jun 4, 2021 that may be closed by this pull request
@DiscJockeyDJ DiscJockeyDJ force-pushed the streamTransaction branch 2 times, most recently from ba4ef23 to 1ae7d18 Compare June 8, 2021 15:10
@DiscJockeyDJ
Copy link
Contributor Author

Stream Transactions are now working as expected. Added good number of cases to test each scenarios.

Let me know if anything is missing.

@DiscJockeyDJ DiscJockeyDJ force-pushed the streamTransaction branch 3 times, most recently from 3b34da2 to 2ed04e2 Compare June 15, 2021 13:56
@DiscJockeyDJ
Copy link
Contributor Author

Added stream transaction support to,

  1. CursorApi --> CursorApiClient.cs
  2. DocumentApi --> DocumentApiClient.cs

For Arango 3.4, filter and exclude the StreamTransaction tests using Xunit Trait.

fix ArangoDB-Community#322
@DiscJockeyDJ
Copy link
Contributor Author

Only TransctionApi changes are now in the PR.

Copy link
Collaborator

@DiscoPYF DiscoPYF left a comment

Choose a reason for hiding this comment

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

@DiscJockeyDJ Thank you for this pull request. I spotted a few things which I think should be addressed before merging. Happy to discuss any of them.

.circleci/config.yml Outdated Show resolved Hide resolved
@DiscJockeyDJ
Copy link
Contributor Author

DiscJockeyDJ commented Jun 21, 2021

Added commit to address the above suggestions 👍

Refactored the TransactionApiClient and its test class.

Created a new class StreamTransactionResponse.

fix ArangoDB-Community#322
Copy link
Collaborator

@DiscoPYF DiscoPYF left a comment

Choose a reason for hiding this comment

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

Thank you for the update. I left another batch of comments which I think are worth addressing before merging, though most of them are fairly minor.

The pull request look good overall, this is a great addition. Looking forward to get it merged 👍

Refactored TransactionApiClientTest and StreamTransactionBody class.

fix ArangoDB-Community#322
@DiscJockeyDJ
Copy link
Contributor Author

Added commit to address the above suggestions 👍

Copy link
Collaborator

@DiscoPYF DiscoPYF left a comment

Choose a reason for hiding this comment

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

@DiscJockeyDJ Thanks for the update. Looks good to me.

@DiscoPYF DiscoPYF merged commit f916d1e into ArangoDB-Community:master Jul 4, 2021
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.

Add support for Stream Transactions
2 participants