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

Ability to handle cold states #18

Merged
merged 9 commits into from
Feb 9, 2021
Merged

Conversation

ivarlokhorst
Copy link
Contributor

No description provided.

@ivarlokhorst
Copy link
Contributor Author

@reinout @valdemetriades

Ik heb een functionaliteit voor het gebruiken van cold state files toegevoegd. Parramatta heeft dit sowieso nodig en ik denk dat dit eigenlijk ook al in texel had moeten zitten.

Reinout zou jij willen reviewen en mergen?

Copy link
Member

@reinout reinout left a comment

Choose a reason for hiding this comment

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

Er moet ook nog een changelog entry bij.

Comment on lines 331 to 351
elif cold_state_id_file.exists():
msg = f"Warm state failed to set; Cold state id file {cold_state_id_file} found"
logger.info(msg)
cold_state_id: str = cold_state_id_file.read_text().strip()
logger.info("Simulation will use initial state %s", cold_state_id)
try:
self.simulations_api.simulations_initial_saved_state_create(
self.simulation_id, data={"saved_state": cold_state_id}
)
return
except openapi_client.exceptions.ApiException as f:
if f.status == 400:
logger.debug("Cold state setting error: %s", str(f))
msg = (
f"Setting initial state to cold state id={cold_state_id} failed. "
f"The error response was {f.body}"
)
raise MissingSavedStateError(msg) from e
else:
logger.debug("Error isn't a 400, so we re-raise it.")
raise
Copy link
Member

Choose a reason for hiding this comment

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

Hier zit heel veel duplicatie in met de code hierboven.
Kan je niet zoiets doen?

for state_file in [saved_state_file, cold_state_file]:
    if not_exist:
        continue
    try/except openapiclient, return als het lukt
if not allow_missing:
    raise MissingSavedStateError

@@ -43,6 +43,7 @@
API_HOST = "https://api.3di.live/v3.0"
CHUNK_SIZE = 1024 * 1024 # 1MB
SAVED_STATE_ID_FILENAME = "3di-saved-state-id.txt"
COLD_STATE_ID_FILENAME = "3di-cold-state-id.txt"
Copy link
Member

Choose a reason for hiding this comment

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

Moet dit niet ook in de readme genoemd worden?

@ivarlokhorst
Copy link
Contributor Author

Ik heb per ongeluk een hele zwik andere commits gedaan. Kan ik 1 versie terug (ik weet niet hoe dit werkt)?

@reinout
Copy link
Member

reinout commented Feb 9, 2021

git revert 883a12b en dan weer pushen. (even copy/pasta doen van de wijzigingen die je wel wilde en die daarna doen).
Misschien zit er in je github windows ding ook een "revert" knop?

@ivarlokhorst
Copy link
Contributor Author

ivarlokhorst commented Feb 9, 2021 via email

fews_3di/simulation.py Outdated Show resolved Hide resolved
)
for state_file in [saved_state_id_file,cold_state_id_file]:
if not state_file.exists():
msg = f"Saved state id file {state_file} not found"
if self.allow_missing_saved_state:
Copy link
Member

Choose a reason for hiding this comment

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

Gaat deze setting erover of er een state file mist? Of dat het toegestaan is dat een staat (niet) meer bestaat op de server?
En zijn ze allebei verplicht?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deze setting gaat over het missen van de state file als bestandje op de computer waar de adapter draait. Later wordt er gekeken via de try except of de state nog beschikbaar is op de 3di server.

fews_3di/simulation.py Outdated Show resolved Hide resolved
fews_3di/simulation.py Outdated Show resolved Hide resolved
raise
raise utils.MissingFileException(msg)
saved_state_id: str = state_file.read_text().strip()
logger.info("Simulation will use initial state %s", saved_state_id)
Copy link
Member

Choose a reason for hiding this comment

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

Om het makkelijker te maken uit te vogelen of het de saved state of de cold state is, is dit misschien nog een handige:

Suggested change
logger.info("Simulation will use initial state %s", saved_state_id)
logger.info("Simulation will use initial state %s from %s", saved_state_id, state_file)

@ivarlokhorst ivarlokhorst merged commit 8674ec0 into master Feb 9, 2021
@ivarlokhorst ivarlokhorst deleted the ivar_cold_state_handling branch February 9, 2021 15:06
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.

2 participants