Skip to content

Fix logging server cleanup #1503

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
Merged
merged 3 commits into from
Jun 15, 2022
Merged

Fix logging server cleanup #1503

merged 3 commits into from
Jun 15, 2022

Conversation

eddiebergman
Copy link
Contributor

@eddiebergman eddiebergman commented Jun 9, 2022 •
edited
Loading

Closes #1480

Tested with the code provided in the issue.

Simply wraps the point of starting the logging server to the point it's cleaned in a try: except

try:
    logger = self._get_logging_server(...)
    ...
finally:
    self.fit_cleanup()
    # Will raise here if something was caught

@eddiebergman eddiebergman mentioned this pull request Jun 9, 2022
Closed
@eddiebergman eddiebergman linked an issue Jun 9, 2022 that may be closed by this pull request
On exception during fit script doesn't terminate cleanly #1480
Closed
@eddiebergman eddiebergman self-assigned this Jun 10, 2022
@eddiebergman eddiebergman added this to the V0.15 milestone Jun 10, 2022
@eddiebergman eddiebergman added the maintenance Internal maintenance label Jun 10, 2022
@eddiebergman
Copy link
Contributor Author

Seems to pass based on the test script provided in #1480 but there's many failing tests which are all timeouts of the test metadata_generation from #1506. I'll rerun the tests and hopefully the PR #1508 will stop issues in the future.

@codecov
Copy link

codecov bot commented Jun 10, 2022 •
edited
Loading

Codecov Report

Merging #1503 (0fb7786) into development (0f1f38a) will increase coverage by 0.02%.
The diff coverage is 78.94%.

@@               Coverage Diff               @@
##           development    #1503      +/-   ##
===============================================
+ Coverage        83.79%   83.81%   +0.02%     
===============================================
  Files              152      153       +1     
  Lines            11667    11694      +27     
  Branches          2037     2047      +10     
===============================================
+ Hits              9776     9801      +25     
  Misses            1343     1343              
- Partials           548      550       +2

Impacted file tree graph

@eddiebergman eddiebergman requested a review from mfeurer June 10, 2022 16:59
mfeurer
mfeurer reviewed Jun 14, 2022
View reviewed changes
@mfeurer mfeurer merged commit 0ae2463 into development Jun 15, 2022
@mfeurer mfeurer deleted the fix_termination_of_autosklearn branch June 15, 2022 06:17
github-actions bot pushed a commit that referenced this pull request Jun 15, 2022
Eddie Bergman: Fix logging server cleanup (#1503)
3b13460
mfeurer pushed a commit that referenced this pull request Jun 17, 2022
@eddiebergman @dependabot
Remove references to validation set in evaluator (#1517)
c69800d 
* Init commit

* Fix logging server cleanup (#1503)

* Fix logging server cleanup

* Add comment relating to the `try: finally:`

* Remove nested try: except: from `fit`

* Bump peter-evans/find-comment from 1 to 2 (#1520)

Bumps [peter-evans/find-comment](https://github.com/peter-evans/find-comment) from 1 to 2.
- [Release notes](https://github.com/peter-evans/find-comment/releases)
- [Commits](peter-evans/find-comment@v1...v2)

---
updated-dependencies:
- dependency-name: peter-evans/find-comment
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump actions/stale from 4 to 5 (#1521)

Bumps [actions/stale](https://github.com/actions/stale) from 4 to 5.
- [Release notes](https://github.com/actions/stale/releases)
- [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md)
- [Commits](actions/stale@v4...v5)

---
updated-dependencies:
- dependency-name: actions/stale
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Init commit

* Update evaluation module

* Clean up other occurences of the word validation

* Re-add test for test predictions

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
eddiebergman added a commit that referenced this pull request Aug 18, 2022
@eddiebergman
Fix logging server cleanup (#1503)
49ea639 
* Fix logging server cleanup

* Add comment relating to the `try: finally:`

* Remove nested try: except: from `fit`
eddiebergman added a commit that referenced this pull request Aug 18, 2022
@eddiebergman @dependabot
Remove references to validation set in evaluator (#1517)
4b40d45 
* Init commit

* Fix logging server cleanup (#1503)

* Fix logging server cleanup

* Add comment relating to the `try: finally:`

* Remove nested try: except: from `fit`

* Bump peter-evans/find-comment from 1 to 2 (#1520)

Bumps [peter-evans/find-comment](https://github.com/peter-evans/find-comment) from 1 to 2.
- [Release notes](https://github.com/peter-evans/find-comment/releases)
- [Commits](peter-evans/find-comment@v1...v2)

---
updated-dependencies:
- dependency-name: peter-evans/find-comment
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump actions/stale from 4 to 5 (#1521)

Bumps [actions/stale](https://github.com/actions/stale) from 4 to 5.
- [Release notes](https://github.com/actions/stale/releases)
- [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md)
- [Commits](actions/stale@v4...v5)

---
updated-dependencies:
- dependency-name: actions/stale
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Init commit

* Update evaluation module

* Clean up other occurences of the word validation

* Re-add test for test predictions

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfeurer mfeurer mfeurer left review comments

Assignees

@eddiebergman eddiebergman

Labels
maintenance Internal maintenance
Projects
None yet
Milestone
V0.15
Development

Successfully merging this pull request may close these issues.

On exception during fit script doesn't terminate cleanly
2 participants
@eddiebergman @mfeurer