search

agrc / forklift

:tractor::package::sparkles: Slinging data all over the place :tractor::package::sparkles:
MIT License
27 stars 3 forks source link

fix: include global id fields in hash #344

Closed stdavis closed 3 years ago

stdavis commented 3 years ago

Description of Changes

This pull request adds global id fields to the change detection hash. Now that we are concerned with them being preserved in the crate destination, we should include them. I'm thinking that the same thing should be done in cambiador. @steveoh, any concerns with making this same change in cambiador?

Test results and coverage

========================================================================================= test session starts ==================================pylint-0.18.0=======================================================                                                                                                                                                
platform win32 -- Python 3.7.10, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 --------
rootdir: W:\forklift, configfile: pytest.ini, testpaths: tests
plugins: instafail-0.4.2, requests-mock-1.8.0, cov-2.12.1, isort-2.0.0,
pylint-0.18.0                                                           --------
collected 216 items
------------------------------------------------------------------------                                                                                                                      [  2/216]--------                                                                                                                                                                                      [  4/216]
Linting files                                                                                                                                                                                 [  6/216]
.................                                                                                                                                                                             [  8/216]
------------------------------------------------------------------------                                                                                                                      [ 10/216]--------                                                                                                                                                                                      [ 12/216]
                                                                                                                                                                                              [ 31/216]
tests\__init__.py ..                                                                                                                                                                          [  2/216]
            [  2/216]                                                                                                                                                                         [  4/216]
tests\PalletWithSyntaxErrors.py ..                                                                                                                                                            [  6/216]
            [  4/216]                                                                                                                                                                         [  8/216]
tests\SchemaLockPallet.py ..                                                                                                                                                                  [ 10/216]
            [  6/216]                                                                                                                                                                         [ 12/216]
tests\benchmark_arcgis.py ..                                                                                                                                                                  [ 31/216]
            [  8/216]                                                                                                                                                                         [ 43/216]
tests\conftest.py ..                                                                                                                                                                          [ 51/216]
            [ 10/216]                                                                                                                                                                         [ 59/216]
tests\mocks.py ..
            [ 12/216]
tests\test_arcgis.py ...................
            [ 31/216]
tests\test_change_detection.py ............
            [ 43/216]
tests\test_changes.py ........
            [ 51/216]
tests\test_config.py ........
            [ 59/216]
tests\test_core.py .......................................
            [ 98/216]
tests\test_crate.py .....................
            [119/216]
tests\test_engine.py ........................s..........
            [154/216]
tests\test_lift.py ...........
            [165/216]
tests\test_messaging.py .......
            [172/216]
tests\test_pallet.py ..................................
            [206/216]
tests\test_seat.py ......
            [212/216]
tests\test_slack.py ....
            [216/216]

========================================================================================== warnings summary ===========================================================================================
c:\users\agrc-arcgis\appdata\local\esri\conda\envs\forklift\lib\site-packages\_pytest\config\__init__.py:1233
  c:\users\agrc-arcgis\appdata\local\esri\conda\envs\forklift\lib\site-packages\_pytest\config\__init__.py:1233: PytestConfigWarning: Unknown config option: log_print

    self._warn_or_fail_if_strict(f"Unknown config option: {key}\n")

-- Docs: https://docs.pytest.org/en/stable/warnings.html

---------- coverage: platform win32, python 3.7.10-final-0 -----------
Name                               Stmts   Miss Branch BrPart     Cover   Missing
---------------------------------------------------------------------------------
src\forklift\arcgis.py               113     29     32      2    70.34%   26, 79-107, 137-138, 143, 146, 163, 178->181, 181, 197->198, 198-200, 222-224
src\forklift\change_detection.py      62      5     16      2    91.03%   32-34, 51->52, 52, 54->55, 55
src\forklift\config.py                50      1     20      2    95.71%   82->83, 83, 118->117
src\forklift\core.py                 198      6     87      5    95.44%   97->100, 100, 129->130, 130, 153-154, 209->212, 272, 303->302, 429->430, 430
src\forklift\engine.py               477    171    160     12    61.54%   133->136, 136, 155-156, 212, 224->225, 225, 237->238, 238-318, 322->368, 332->333, 333-335, 358-359, 397-468, 498, 508, 522-538, 543-556, 571->572, 572, 592, 610->611, 611, 624-627, 678->681, 681-690, 718->721, 721-722, 724->727, 727, 741-747, 753, 757->760, 760, 805->806, 806, 911-954
src\forklift\lift.py                 148     52     52      4    61.00%   34, 41->42, 42, 152->153, 153-156, 159->163, 163-164, 187-198, 233-254, 281, 284->285, 285, 292-295, 310-317, 346-357
src\forklift\messaging.py             53     10     16      1    72.46%   58, 78->79, 79, 100-112
src\forklift\models.py               196      4     52      3    96.37%   96, 183->182, 404->407, 407, 451->459, 459-460
src\forklift\slack.py                211     33    102     33    75.72%   24->25, 25-27, 54->57, 57, 71->72, 72-77, 79->80, 80-85, 90->93, 100->101, 101, 102->103, 103, 104->105, 105, 111->114, 116->117, 117-118, 120->87, 134->137, 137, 151->184, 155->158, 160->161, 161-164, 165->168, 168->171, 174->177, 189->192, 195->197, 197->200, 202->205, 236, 259->260, 260, 265, 276->278, 278->279, 279, 284->287, 287->288, 288, 304->305, 305, 331->332, 332, 333->334, 334, 347->348, 348, 351->354, 354->357, 360, 372, 375
---------------------------------------------------------------------------------
TOTAL                               1530    311    543     64    76.22%

3 files skipped due to complete coverage.

======================================================================== 215 passed, 1 skipped, 1 warning in 598.01s (0:09:58) ========================================================================

Speed test results

Speed Test Results
Dry Run: 2.73 minutes
Repeat: 2.16 minutes
steveoh commented 3 years ago

I don't believe cambiador needs changing. Are you seeing otherwise?

stdavis commented 3 years ago

I see globalid and global_id in skipFields: https://github.com/agrc/cambiador/blob/622bd07b9d532d50c3881e7dfddb128cfef5f631/Program.cs#L114.

steveoh commented 3 years ago

ahh, sorry, i was thinking this was the forklift hash stuff.

stdavis commented 3 years ago

I've pushed this to the server.