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

[CIVIS-1822] ENH file_id_from_run_output works for all job types #461

Merged
merged 9 commits into from
Feb 8, 2023

Conversation

KasiaHinkson
Copy link
Contributor

@KasiaHinkson KasiaHinkson commented Feb 8, 2023

Replace client.scripts.list_containers_runs_outputs with client.jobs.list_runs_outputs in civis.io.file_id_from_run_output so it works for all job types, not just container scripts.

  • (For Civis employees only) Reference to a relevant ticket in the pull request title
  • Changelog entry added to CHANGELOG.md at the repo's root level
  • Description of change in the pull request description
  • If applicable, unit tests have been added and/or updated
  • The CircleCI builds have all passed

Copy link
Member

@jacksonlee-civis jacksonlee-civis left a comment

Choose a reason for hiding this comment

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

@KasiaHinkson Could you rebase the upstream "master" branch into your branch here? That should resolve the build failure.

@KasiaHinkson
Copy link
Contributor Author

Done, that worked. Thank you!

@jacksonlee-civis jacksonlee-civis changed the title CIVIS-1822 run outputs [CIVIS-1822] ENH file_id_from_run_output works for all job types Feb 8, 2023
@jacksonlee-civis jacksonlee-civis added this to the v2.0.0 milestone Feb 8, 2023
Copy link
Member

@jacksonlee-civis jacksonlee-civis left a comment

Choose a reason for hiding this comment

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

@KasiaHinkson Thank you for improving civis-python! Two small changes are needed (see my comments below), then this PR should be good to go.

civis/parallel.py Outdated Show resolved Hide resolved
civis/tests/mocks.py Outdated Show resolved Hide resolved
Copy link
Member

@jacksonlee-civis jacksonlee-civis left a comment

Choose a reason for hiding this comment

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

LGTM!

@KasiaHinkson KasiaHinkson merged commit 1ff8bda into master Feb 8, 2023
@KasiaHinkson KasiaHinkson deleted the civis-1822-run-outputs branch February 8, 2023 18:55
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.

2 participants