add node argument to batch method

[sinisaos]

Related to #578

[dantownsend]

@sinisaos This is great - thanks!

We just need to add a test somehow. Maybe we can add an extra test to this file:

https://github.com/piccolo-orm/piccolo/blob/master/tests/engine/test_extra_nodes.py

It will be very similar, it just uses batch.

[sinisaos]

@dantownsend Will this be enough?

from piccolo.utils.sync import run_sync

# Make sure the node is queried in batch
run_sync(Manager.select().batch(node="read_1"))
self.assertTrue(EXTRA_NODE.run_querystring.called)

Tests pass with it.

[dantownsend]

@sinisaos Yes, I think that's OK.

[codecov-commenter]

Codecov Report

Merging #579 (775c0be) into master (0c44bb2) will increase coverage by 0.39%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #579      +/-   ##
==========================================
+ Coverage   90.82%   91.21%   +0.39%     
==========================================
  Files         105      105              
  Lines        7183     7038     -145     
==========================================
- Hits         6524     6420     -104     
+ Misses        659      618      -41     

Impacted Files	Coverage Δ
piccolo/apps/fixtures/commands/shared.py	100.00% <ø> (+4.54%)	:arrow_up:
piccolo/querystring.py	92.20% <ø> (+1.06%)	:arrow_up:
piccolo/table.py	95.87% <ø> (+0.23%)	:arrow_up:
piccolo/utils/objects.py	93.75% <ø> (+4.86%)	:arrow_up:
piccolo/utils/sql_values.py	94.44% <ø> (+4.44%)	:arrow_up:
piccolo/engine/base.py	89.36% <100.00%> (+15.22%)	:arrow_up:
piccolo/engine/postgres.py	91.38% <100.00%> (+0.04%)	:arrow_up:
piccolo/engine/sqlite.py	96.63% <100.00%> (+0.38%)	:arrow_up:
piccolo/query/methods/objects.py	97.05% <100.00%> (+0.04%)	:arrow_up:
piccolo/query/methods/select.py	99.57% <100.00%> (+<0.01%)	:arrow_up:
... and 35 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[dantownsend]

I just realised Objects and Select have a batch method, which needs to accept the node argument, and pass it through to the Engine.batch method.

[sinisaos]

I'm pass a test database in test_batch.py as a read_replica to be able to test if batch method node argument is called. Locally all tests passed but failed on Sqlite in GH actions.

[sinisaos]

@dantownsend I was finally able to pass all the tests in GH actions by importing the test database instance into the local namespace of the test method. Can you check if that is ok?

[dantownsend]

@sinisaos Yeah, it's tricky to test. I'm having a look.

[dantownsend]

@sinisaos I changed the testing approach slightly. What you did was good. The only problem is we were modifying the global DB variable - which would probably be OK, but I changed it so we use a local DB variable, so there's less chance of other tests being effected.

I see that you added some coverage config to pyproject.toml - it's a good idea. I just removed 'ValueError' because I'm worried there may be some legitimate tests which might be missing - all ValueError should be covered really.

[sinisaos]

@dantownsend Your approach is much better, and everything looks good. Thanks.