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

deps: remove aiofiles #5308

Merged
merged 1 commit into from
Jan 18, 2023
Merged

deps: remove aiofiles #5308

merged 1 commit into from
Jan 18, 2023

Conversation

oliver-sanders
Copy link
Member

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@oliver-sanders oliver-sanders added this to the cylc-8.2.0 milestone Jan 17, 2023
@oliver-sanders oliver-sanders self-assigned this Jan 17, 2023
* Aiofiles is a simple wrapper for a thread pool executor.
  We don't need a 3rd party library to do this for us.
* Closes cylc#5306
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

LGTM. I have done a quick test of playing + cleaning a workflow, TUI, and running the fast tests locally

Comment on lines -655 to -662
async for line in cont:
key, value = [item.strip() for item in line.split("=", 1)]
# BACK COMPAT: contact pre "suite" to "workflow" conversion.
# from:
# Cylc 8
# remove at:
# Cylc 8.x
data[key.replace('SUITE', 'WORKFLOW')] = value
Copy link
Contributor

Choose a reason for hiding this comment

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

Will removing this catch anyone out?

Copy link
Member

Choose a reason for hiding this comment

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

It was a duplicate of what was already in load_contact_file() so not actually removed

Copy link
Member Author

Choose a reason for hiding this comment

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

I've replaced this with the sync load_contact_file interface run inside a thread via make_async.

@datamel
Copy link
Contributor

datamel commented Jan 18, 2023

Flaky tests failing with a codecov upload so unrelated to this pr.

@datamel datamel merged commit fb63772 into cylc:master Jan 18, 2023
@oliver-sanders oliver-sanders deleted the drop-aiofiles branch January 18, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dependencies: drop aiofiles
3 participants