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.

Sign up for GitHub

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

Jump to bottom

Mitigate next_etrago_id collisions #927

Open
wants to merge 16 commits into
base: dev
Choose a base branch
from
Open

Mitigate next_etrago_id collisions #927

wants to merge 16 commits into from

Commits on Sep 7, 2022

  1. Update the pre-commit configuration 

    Done simply via `pre-commit autoupdate`. Should fix all pre-commit
issues.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    049b185 View commit details
    Browse the repository at this point in the history

  2. Tell flake8 that this bare except is OK 

    Otherwise it would complain with "E722 do not use bare 'except'".
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    3ad23ba View commit details
    Browse the repository at this point in the history

  3. Use (parts of) a UUID4 as the next_etrago_id 

    I did a few experiments and using six bytes of the UUID would've been OK
but risky, but using seven bytes didn't lead to collisions for up to
around 1.6e+10 UUIDs in around 3e+5 contiguous ranges with random
lengths from 1e+4 to 1e+5. Using eight bytes would mean the risk of
generating integers which are to big for PostgreSQL's BigInt datatype,
so seven bytes is the best compromise between simplicity and collision
mitigation.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    da7703b View commit details
    Browse the repository at this point in the history

  4. Avoid a bare except 

    This is due to a "flake8" complaint, but it also makes the code more
explicit. The intention is to provide an alternative, should the
`dataset_2040[node]` lookup fail. Explicitly catching only the
`KeyError` caused by this avoids potentially hiding other errors.
There's still a bug in this code, as explained in issue #926, but that's
something which has to be fixed separately.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    0cb29fc View commit details
    Browse the repository at this point in the history

  5. Fix style and clean up the code 

    Most of these are done automatically via "isort" or "black". The
exceptions are:

  - fixing the line length of SQL strings and comments,
  - removing trailing whitespace in SQL strings and
  - removing the unused local variable `sources`.

These things had to be done manually to ensure they really don't change
the semantics of the code.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    7876b1f View commit details
    Browse the repository at this point in the history

  6. Reformat "emobility/**/model_timeseries.py" 

    There's some interesting stuff in here. First, some of the lines, the
ones within the `# fmt: on`/`# fmt: off` blocks, could not be formatted
by "black" due to psf/black#510. Second, I had to wrestle with "isort"
quite a bit because it kept formatting the imports to be too long so I
had to resort to the `# noqa: E501` solution. Last but not least, if an
import is only used in a docstring, "flake8" doesn't seem to recognize
it as used.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    81bca5f View commit details
    Browse the repository at this point in the history

  7. Stop incrementing next_etrago_id results 

    Since the IDs are randomly generated now, there's no reason to increment
them anymore. Both variants are (roughly) equally likely to collide with
IDs already in use.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    91af5d2 View commit details
    Browse the repository at this point in the history

  8. Fix various style issues in "datasets/gas_grid.py" 

    Mostly overly long lines but also one missing space after an inline
comment (`#`) sign and one f-string without placeholders.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    cdc3220 View commit details
    Browse the repository at this point in the history

  9. Remove superfluous casts to int 

    The result of `next_etrago_id` is now always an `int` so no need to cast
it.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    ed6e456 View commit details
    Browse the repository at this point in the history

  10. Fix import order and a trailing space

    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    4ad629d View commit details
    Browse the repository at this point in the history

  11. Move next_etrago_id calls closer to use sites 

    At least where possible. I also took the liberty of removing comments
like `# Select next id value` because these just restate the name of the
function in other words so one could argue that they should fall into a
category of comments which should be avoided according to [PEP8][0].

[0]: https://peps.python.org/pep-0008/#inline-comments
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    7c1a2ac View commit details
    Browse the repository at this point in the history

  12. Remove unused local variables 

    As requested by "flake8".
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    99c2b15 View commit details
    Browse the repository at this point in the history

  13. Rename max_id to new_id 

    Since we now generate IDs randomly, they are no longer "guaranteed" to
be the current maximum so the old name is misleading.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    a223112 View commit details
    Browse the repository at this point in the history

  14. Remove empty lines 

    The respective code blocks kind of belong together and having them
closer to each other makes this a little bit more obvious. See also the
[PEP8 note][0] on using "blank lines [..] sparingly".

[0]: https://peps.python.org/pep-0008/#blank-lines
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    085448a View commit details
    Browse the repository at this point in the history

  15. Fix potentially dangerous duplicate calls 

    Calling `next_etrago_id` two times in "close" succession did not
guarantee getting the same results even before switching to random IDs.
This was due to concurrency. Now that we're using random IDs, these two
calls are basically guaranteed to yield wildly different results (which
is the point of the new implementation), so this pattern is very
dangerous and will nearly always be wrong.
Fortunately it is easily fixed by saving the result of the first call
and using it in place of both calls.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    5593c82 View commit details
    Browse the repository at this point in the history

  16. Fix circular import 

    Turns out that it's not possible to import `MotorizedIndividualTravel`
because it leads to a circular import. Solution:

```
import egon.data.datasets.emobility.motorized_individual_travel
```

instead and pull `MotorizedIndividualTravel` out of it at use time.
    @gnn
    gnn committed Sep 7, 2022
    Configuration menu
    Copy the full SHA
    c194284 View commit details
    Browse the repository at this point in the history