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

[DE-664] Deprecating Batch API #279

Merged
merged 5 commits into from
Sep 15, 2023
Merged

Conversation

apetenchea
Copy link
Member

@apetenchea apetenchea commented Sep 1, 2023

https://www.arangodb.com/docs/stable/http/batch-request.html

  • The batch API is deprecated since ArangoDB 3.8.0 and will be removed in a future version.
  • The BatchDatabase is still available, but it now uses a TreadPoolExecutor internally.
  • For backwards compatibility, the BatchDatabase uses only one worker thread, essentially sending the requests sequentially. Feel free to set the max_workers parameter to a higher value if you want to use multiple threads, but be aware that the requests will be sent in parallel, which may cause problems if you are using transactions.
  • To discourage the use of this API, we now issue a warning when the BatchDatabase is used. This warning may be disabled by the user.

Note that {"foo": "bar"} may be inserted after {"foo": "baz"} in the following example:

with db.begin_batch_execution(max_workers=2) as batch_db:
    job1 = batch_db.collection.insert({"foo": "bar"})
    job2 = batch_db.collection.insert({"foo": "baz"})

Documentation has been updated accordingly.

@apetenchea apetenchea self-assigned this Sep 1, 2023
@@ -144,28 +133,3 @@ def test_batch_job_result_not_ready(db, col, docs):
assert batch_db.commit() == [job]
assert len(job.result()) == len(docs)
assert extract("_key", col.all()) == extract("_key", docs)


def test_batch_bad_state(db, col, docs):
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this test no longer makes sense with the current implementation, because we don't throw this exception anymore.

@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2023

Codecov Report

Merging #279 (3752307) into main (744a6e9) will decrease coverage by 0.38%.
Report is 12 commits behind head on main.
The diff coverage is 93.54%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #279      +/-   ##
==========================================
- Coverage   99.01%   98.64%   -0.38%     
==========================================
  Files          26       26              
  Lines        3958     3981      +23     
==========================================
+ Hits         3919     3927       +8     
- Misses         39       54      +15     
Files Changed Coverage Δ
arango/client.py 98.41% <ø> (ø)
arango/http.py 100.00% <ø> (ø)
arango/collection.py 97.59% <91.39%> (-0.92%) ⬇️
arango/database.py 99.72% <100.00%> (-0.14%) ⬇️
arango/exceptions.py 100.00% <100.00%> (ø)
arango/executor.py 99.35% <100.00%> (-0.13%) ⬇️
arango/job.py 100.00% <100.00%> (ø)
arango/request.py 94.44% <100.00%> (ø)
arango/utils.py 87.50% <100.00%> (-12.50%) ⬇️

@apetenchea apetenchea added the Ready to Merge Pull request is ready to merge label Sep 8, 2023
Copy link
Member

@aMahanna aMahanna left a comment

Choose a reason for hiding this comment

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

LGTM

@aMahanna aMahanna merged commit 371be70 into main Sep 15, 2023
7 checks passed
@aMahanna aMahanna deleted the feature/de-664-modernizing-batch-api branch September 15, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to Merge Pull request is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants