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

Fix cancel #463

Merged
merged 6 commits into from
Feb 28, 2019
Merged

Fix cancel #463

merged 6 commits into from
Feb 28, 2019

Conversation

anyaelena
Copy link
Member

Fixes #437

@coveralls
Copy link

coveralls commented Feb 21, 2019

Coverage Status

Coverage increased (+0.05%) to 66.401% when pulling b1cae51 on fix-cancel into ead3e41 on develop.

Copy link
Member

@nllong nllong left a comment

Choose a reason for hiding this comment

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

Looks good!

image

Copy link
Member

@nllong nllong left a comment

Choose a reason for hiding this comment

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

One of the travis test failures is an actual issue. I'll look into it.

@nllong
Copy link
Member

nllong commented Feb 26, 2019

One of the travis test failures is an actual issue. I'll look into it.

@anyaelena -- actually is there a way you can fix the test?

@anyaelena
Copy link
Member Author

@nllong yes, i'll look into it.

@nllong
Copy link
Member

nllong commented Feb 26, 2019

@anyaelena thanks. I think this can be merged down once you fix the failing test.

@anyaelena
Copy link
Member Author

@nllong i verified locally that complete is working as expected. i don't have the test working but my working theory is that the introduction of the after_enqueue hook is messing with the test setup.

@nllong
Copy link
Member

nllong commented Feb 28, 2019

@anyaelena - i fixed the test to have it check for something else to ensure completion. I'm pretty sure that the queue is not getting updated because the simulations are not actually running in a queue so any triggers are probably ignored. (https://github.com/NREL/OpenStudio-server/pull/463/files#diff-0a351848e90287c4b763ccbc56e78ea3R143).

@nllong nllong merged commit e63abbe into develop Feb 28, 2019
@nllong nllong deleted the fix-cancel branch February 28, 2019 22:32
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.

3 participants