Skip to content

Conversation

@tsibley
Copy link
Contributor

@tsibley tsibley commented Feb 17, 2023

See commit messages for details.

Testing

  • Manual testing of behaviour locally
  • Checks pass

@tsibley tsibley requested a review from a team February 17, 2023 23:43
Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

The tests for Python 3.6 are failing...do we still need to support 3.6?

@corneliusroemer
Copy link
Member

The tests for Python 3.6 are failing...do we still need to support 3.6?

It's btw not the PR that's causing tests to fail - CI fails also on master: https://github.com/nextstrain/cli/actions/runs/4266385449

I've made a PR to bump minimum supported Python version to 3.7: #256

…lready complete

This messaging and signal handling is more confusing than helpful when
attaching to a job that's already complete (i.e. in a terminal status).
For such jobs, just exit normally when receiving a ^C (as if
"detaching").
It can be annoying to sit thru the fetching and printing of large reams
of log lines when attaching to a completed AWS Batch build to download
the results.  This is particularly true when downloading small bits of
the results via --download, as the log fetching often takes longer than
the selective download does!
@tsibley tsibley force-pushed the trs/aws-batch/attaching-to-complete-jobs branch from 1470bcd to f1f491b Compare February 28, 2023 18:33
@tsibley
Copy link
Contributor Author

tsibley commented Feb 28, 2023

Rebased onto latest master to pick up fixes for CI (and fix changelog conflicts).

@tsibley tsibley merged commit 9249a4a into master Feb 28, 2023
@tsibley tsibley deleted the trs/aws-batch/attaching-to-complete-jobs branch February 28, 2023 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

4 participants