-
Notifications
You must be signed in to change notification settings - Fork 61
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
Repair OLIAPI #1512
base: main
Are you sure you want to change the base?
Repair OLIAPI #1512
Conversation
…solver iterations; also replacing aeration tank on bsm2-p instead of cstr_injection with flowsheet level vars and constraints
…eys; revisit this change
… BSM2-P solver iterations; also replacing aeration tank on bsm2-p instead of cstr_injection with flowsheet level vars and constraints" This reverts commit bd2caa8.
…ing files; fix issue related to session dbs file deletion on exit
…credentials used; bring tests back
The OLI API CI failures are not entirely reproducible. @adam-a-a will reach out to OLI to investigate this. |
… to troubleshoot errors after switch to sequential test deployment
…hange would break them again to identify what the solution was
…d bring back a change that I ultimately want to remove but think might have been part of prob
…erted for oliapi_instance, along with point to last session dbs file in test_flash; now will set refresh=True on oliapi_instance in conftest
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1512 +/- ##
==========================================
+ Coverage 93.51% 95.01% +1.50%
==========================================
Files 282 282
Lines 29933 29958 +25
==========================================
+ Hits 27991 28465 +474
+ Misses 1942 1493 -449 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but CodeCov is failing mostly as a result of the new logger and error messages being untested
…sponse when shifting to alternate version of OLI used for NAWI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - the tutorial runs fine on my end now. I just have one small comment...
if self.interactive_mode: | ||
msg = msg + "Enter [y]/n to proceed." | ||
return input(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the interactive mode functionality work in Jupyter? It doesn't seem to be the case, and, if so, perhaps it should be set to False in the tutorial
…for credentials encryption in interactive mode; make effort to ensure "refresh" option works when enabled and username/password used for login
…ertap into repair_oliapi_oct24
…torial per Marcus' suggestion
Fixes/Resolves:
OLI API stopped working.
Summary/Motivation:
Users were no longer able to get back OLI calculations. We were able to log in, generate a DBS file, and that was about it. The issue came down to an additional forward slash used in one of the URLs, which previously worked.
EDIT: Subsequently, CI checks were failing as well, while local tests were passing. This PR also addresses those issues.
Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: