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

PVOutput exports schedulen naar ingestelde upload interval #441

Merged
merged 2 commits into from Feb 26, 2018

Conversation

pyrocumulus
Copy link
Contributor

Deze PR lost een klein schoonheidsfoutje op in de PVOutput export. Af en toe laat de export namelijk een gat vallen aan de kant van PVO en dat ziet er dan zo uit:

image

Na wat pluiswerk in de logs, kwam ik tot de conclusie dat de upload tijd uiteindelijk een beetje gaat verschuiven in seconden en niet exact altijd 5 minuten is als je dat ingesteld hebt. De timestamp van de upload wordt daarnaast afgerond op hele minuten, waardoor de hele tijd ineens met een minuut aangepast wordt. Dit veroorzaakt timings als:

  1. PVOutput upload om 15:17
  2. PVOutput upload om 15:22 (~5 minuten verschil)
  3. PVOutput upload om 15:28 (~5 minuten verschil, maar naar boven afgerond)

Dit betekent dat de uploads aan de kant van PVO ook anders afgerond worden, namelijk naar respectievelijk: 15:15, 15:20 en 15:30. Je mist in dit scenario dus de 15:25 upload. Ik heb meerdere van deze situaties bekeken en vergeleken met de log en de oorzaak lijkt elke keer zo'n versprongen minuut te zijn.

In deze PR die ik inmiddels al een paar dagen lokaal gebruik, rond ik het schedule tijdstip af naar het ingestelde aantal minuten maar dan alvast aan de kant van DSMR. De timing kan nog altijd wel een paar seconden afwijken maar dit verschil wordt elke keer weg 'afgerond'. Dit is voldoende om het probleem op te lossen, denk ik.

Ik ben geen Python eindbaas noch Django maar ik heb wel m'n best er op gedaan. Kon dit alleen testen door de code op mijn productie dsmr te draaien eigenlijk en heb alleen met mijn interval van 5 minuten getest (maar ik zie niet hoe dat een probleem kan zijn). Kijk er dus nog even goed naar zou ik zeggen 👍 Maar wellicht dat anderen dit verschijnsel ook af en toe bij PVO zien.

@pyrocumulus pyrocumulus changed the base branch from master to development February 22, 2018 08:25
Opgelost:
dsmr_pvoutput/services.py:30: [W] W293 blank line contains whitespace [pycodestyle]
dsmr_pvoutput/services.py:36: [C] E711 comparison to None should be 'if cond is None:' [pycodestyle]
@pyrocumulus
Copy link
Contributor Author

Duurde beetje lang, ik had geen notificatie gekregen van de falende Travis CI build (weet niet of dat überhaupt gebeurt trouwens). Bij deze zijn de fouten daarvan gefixed. Zou nu goed moeten zijn voor zover ik kan zien.

@codecov-io
Copy link

codecov-io commented Feb 22, 2018

Codecov Report

Merging #441 into development will decrease coverage by 0.08%.
The diff coverage is 77.77%.

@@               Coverage Diff               @@
##           development     #441      +/-   ##
===============================================
- Coverage          100%   99.91%   -0.09%     
===============================================
  Files               96       96              
  Lines             2318     2327       +9     
  Branches           183      184       +1     
===============================================
+ Hits              2318     2325       +7     
- Misses               0        1       +1     
- Partials             0        1       +1

@codecov-io
Copy link

codecov-io commented Feb 22, 2018

Codecov Report

Merging #441 into development will not change coverage.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           development   #441   +/-   ##
==========================================
  Coverage          100%   100%           
==========================================
  Files               96     96           
  Lines             2318   2324    +6     
  Branches           183    183           
==========================================
+ Hits              2318   2324    +6

@dennissiemensma
Copy link
Member

Bedankt voor je request! Het klinkt inderdaad vrij aannemelijk dat de timing telkens een paar seconden verschuift. Ik zal er even naar kijken.

Mocht je trouwens de mogelijkheid hebben om te kunnen ontwikkelen op een Linux-machine, dan is het vrij makkelijk om een ontwikkelomgeving op te zetten (ik heb alleen de stappen daarvoor nog niet concreet gedocumenteerd). Mocht je er behoefte aan hebben.

@pyrocumulus
Copy link
Contributor Author

Volgens mij kan ik wel een VM de lucht in schoppen met Linux erin voor ontwikkeling. Op zich is dat nog niet het grootste probleem denk ik, het is meer de omgeving eromheen dus een dev-database met toch wat inkomende data vanuit een slimme meter. Ik zag niet helemaal voor me hoe ik dat even snel kon opzetten hiervoor :-) De python code zelf schreef prima in Visual Studio Code; moet alleen even een goede linter installeren.

Maar inderdaad, een guide met hoe een ontwikkelomgeving op te zetten voor dsmr is wel praktisch denk ik. Dan zijn ook complexere punten te tackelen, buiten steeds testen op m'n 'productie' dsmr installatie dan ;-)

@dennissiemensma
Copy link
Member

Ik heb het nu volgens mij simpeler voor je opgelost met alleen modulo-aftrekken op basis van de interval.

Wil je eens meekijken in de tests of ik je probleem en oplossing goed heb begrepen?

  • Test 1: Draait om uur xx:13, interval 5 minuten, uitkomst is uur xx:15 (want 13 + 5 = 18, en dan afronden naar beneden)

  • Test 2: Draait om uur xx:25, interval 15 minuten, uitkomst is uur xx:30 (want 25 + 15 = 40, en dan afronden naar beneden)

  • Test 3: Draait om uur xx:59, interval 15 minuten, uitkomst is uur xx:00 (want 59 + 15 = 14, en dan afronden naar beneden)

@pyrocumulus
Copy link
Contributor Author

pyrocumulus commented Feb 22, 2018

Ik had ook aan deze aanpak gedacht maar ik dacht dat een modulo methode niet kon werken omdat je over het hele uur of zelfs dag heen zou kunnen gaan. Je tests testen volgens mij het juiste probleem plus assert maar wat zou de volgende casus doen met de huidige oplossing:

  • Test 4: Draait om uur 6:53, interval 5 minuten, uitkomst is uur: 7:00 (want 53 + 5 = 58, afronden naar boven). Ofwel, een verspringing van het uur.

Gebeurt dat wel met de next_export.replace(...) methode? Dit zou in theorie ook rond het uur 0:00 kunnen spelen en dan met een verspringing van de dag/maand/jaar tot gevolg. Als ik* het goed begrijp zou je nu een next_export timestamp kunnen creëren die in het verleden ligt.

*= Het zou kunnen dat deze casus ook gedekt is en dat ik dat niet zie vanwege mijn wat mindere Python kennis hoor. Maar deze situatie was voor mij de reden voor de ietwat complexere aanpak 🙂

@dennissiemensma
Copy link
Member

dennissiemensma commented Feb 22, 2018

Bedankt voor je terugkoppeling. De huidige oplossing rond niet af naar boven en dat zou ook niet nodig moeten zijn, dus test 4 gaat niet werken in dit geval. Hij gaat altijd terug naar het eerstvorige punt en dat is altijd vooruit, ten opzichte van 'nu' (omdat hij eerst nu + interval doet).

De backend wordt doorgaans elke seconde aangeroepen en is daarmee vrijwel gegarandeerd dat die telkens minstens één keer binnen de kleinste interval, van 5 minuten, draait en daarmee zou bovenstaande oplossing moeten werken.
Uitzonderingen zijn langdraaide runs, zoals het maken van de database-backup en upload naar dropbox. Echter, dat probleem blijft ook bestaan bij afronden naar boven, want er kan eenmaal een gat van X minuten komen daardoor. De backend wordt gedurende de backup of upload vastgehouden door dat proces.

Je oorspronkelijke probleem zou hierdoor echter niet meer voor moeten komen, er van uitgaande dat de backend minstens 1x per interval (van minimaal 5 minuten) aangeroepen wordt en er daardoor geen gat ontstaat. De secondewijzer wordt daarnaast ook nog eens hard gereset.
De .replace() vervangt letterlijk de waarde en voor de seconde zet ik die op nul en voor de minuut gaat dat goed door de modulo (hij gaat eerst naar het volgende uur en doet dan pas de modulo, dus resultaat is daar altijd nul).
Test 3 heeft een verspringing van het uur en ik heb nu ook nog wat checks in de tests toegevoegd die het uur + seconde controleren. Zie de nieuwe versie in 6febab4.

Hopelijk begrijp je wat ik bedoel. Het is wat lastig uit te typen. Wellicht wil je anders de branch pyrocumulus-pvoutput-schedule een dagje thuis uitproberen voordat ik hem definitief doorvoer?

@goegol
Copy link
Contributor

goegol commented Feb 22, 2018 via email

@pyrocumulus
Copy link
Contributor Author

Ah ik had over het hoofd gezien dat je nooit naar boven afrond; dan klopt je oplossing inderdaad en dat maakt alles wel een stuk simpeler :) Je aangepaste tests zijn prima, stukje explicieter op deze manier door hele timestamp te verifiëren. Je verhaal is verder duidelijk hoor!

Ik wil de branch wel even testen hoor; zou je me wel de commando's kunnen geven waarmee ik dat tijdelijk in mijn installatie kan doen? Dat soort dingen heb ik nog niet vaak gepoogd met git namelijk 🙂

@dennissiemensma
Copy link
Member

Als het goed is:

sudo su - dsmr
git fetch
git checkout -b pyrocumulus-pvoutput-schedule origin/pyrocumulus-pvoutput-schedule
./deploy.sh

Terug kan met:

sudo su - dsmr
# Master of welke andere branch je maar wilt
git checkout master
./deploy.sh

@pyrocumulus
Copy link
Contributor Author

Oh dat is geheel niet moeilijk. Ga kijken of ik dat vanavond even kan fixen, dan kan ik het weekend mooi testen.

Ik laat het je weten!

@pyrocumulus
Copy link
Contributor Author

Hmm, ik krijg het volgende na het switchen naar de andere branch:

Traceback (most recent call last):
  File "/home/dsmr/dsmr-reader/manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/dsmr/.virtualenvs/dsmrreader/lib/python3.4/site-packages/django/core/management/__init__.py", line 371, in execute_from_command_line
    utility.execute()
  File "/home/dsmr/.virtualenvs/dsmrreader/lib/python3.4/site-packages/django/core/management/__init__.py", line 317, in execute
    settings.INSTALLED_APPS
  File "/home/dsmr/.virtualenvs/dsmrreader/lib/python3.4/site-packages/django/conf/__init__.py", line 56, in __getattr__
    self._setup(name)
  File "/home/dsmr/.virtualenvs/dsmrreader/lib/python3.4/site-packages/django/conf/__init__.py", line 43, in _setup
    self._wrapped = Settings(settings_module)
  File "/home/dsmr/.virtualenvs/dsmrreader/lib/python3.4/site-packages/django/conf/__init__.py", line 106, in __init__
    mod = importlib.import_module(self.SETTINGS_MODULE)
  File "/home/dsmr/.virtualenvs/dsmrreader/lib/python3.4/importlib/__init__.py", line 109, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 2254, in _gcd_import
  File "<frozen importlib._bootstrap>", line 2237, in _find_and_load
  File "<frozen importlib._bootstrap>", line 2224, in _find_and_load_unlocked
ImportError: No module named 'dsmrreader.settings'

Processen blijven daarop vastlopen. Terug switchen naar master helpt helaas ook niet, ook tijdens de ./deploy .sh krijg ik daarop errors.

@pyrocumulus
Copy link
Contributor Author

Damn, ik weet het denk ik al. Ik heb op de verkeerde manier m'n lokale changes ongedaan gemaakt. Ik ben letterlijk m'n settings.py file kwijt.

@pyrocumulus
Copy link
Contributor Author

Oké, opgelost. Sorry voor de spam ;-) Je kunt bovenstaande veilig negeren. M'n pi draait nu de test branch voor het komende weekend!

Note to self: git clean -fdx is bijzonder rigoureus.

@dennissiemensma
Copy link
Member

dennissiemensma commented Feb 23, 2018 via email

@pyrocumulus
Copy link
Contributor Author

pyrocumulus commented Feb 23, 2018 via email

@pyrocumulus
Copy link
Contributor Author

Branch heeft het hele weekend goed gedraaid hoor. Doorgaans had ik in zo'n periode al minimaal één ontbrekend uploadmoment bij PVO gehad, dus ik beschouw het als opgelost 👍

@dennissiemensma dennissiemensma merged commit e094203 into dsmrreader:development Feb 26, 2018
@dennissiemensma
Copy link
Member

Bedankt voor je terugkoppeling! Ik zal mijn versie dan voor je mergen naar development. Je kunt dan eventueel die branch draaien (of de huidige behouden tot de volgende release).

@pyrocumulus pyrocumulus deleted the pvoutput-schedule branch February 27, 2018 11:40
@dennissiemensma dennissiemensma added this to the 1.14 milestone Mar 5, 2018
@dennissiemensma
Copy link
Member

Uitgebracht in de nieuwe release.

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.

None yet

4 participants