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.

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

Fix running CML inside CML with pkg binaries #458

Merged
merged 6 commits into from
Mar 25, 2022
Merged

Fix running CML inside CML with pkg binaries #458

merged 6 commits into from
Mar 25, 2022

Conversation

0x2b3bfa0
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 commented Mar 24, 2022 •
edited
Loading

Fixes iterative/cml#920, but is just a hack; don't merge unless we don't find a better solution.

@0x2b3bfa0 0x2b3bfa0 marked this pull request as ready for review March 24, 2022 02:19
@0x2b3bfa0 0x2b3bfa0 self-assigned this Mar 24, 2022
@0x2b3bfa0 0x2b3bfa0 added bug Something isn't working resource-runner iterative_runner TF resource labels Mar 24, 2022
@0x2b3bfa0 0x2b3bfa0 requested a review from a team March 24, 2022 02:20
@0x2b3bfa0 0x2b3bfa0 mentioned this pull request Mar 24, 2022
Closed
casperdcl
casperdcl suggested changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

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

don't merge unless we don't find a better solution

this isn't not double negation backfiring

iterative/utils/helpers.go Outdated Show resolved Hide resolved
@0x2b3bfa0
Copy link
Member Author

I'm not denying that this pull request must not be left unmerged.

@0x2b3bfa0 0x2b3bfa0 requested a review from casperdcl March 24, 2022 22:17
casperdcl
casperdcl reviewed Mar 25, 2022
View reviewed changes
Copy link
Contributor

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

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

okay but this level of hackery requires an inline comment methinks

0x2b3bfa0 and others added 2 commits March 25, 2022 01:20
@0x2b3bfa0 @casperdcl
Add inline comment
b49bfd2 
Co-authored-by: Casper da Costa-Luis <[email protected]>
@0x2b3bfa0
DWIM!
cc08781
@dacbd
Copy link
Contributor

dacbd commented Mar 25, 2022

The alternative route I was pursuing was (partially) unsuccessful, it might still be possible but it requires more tinkering.

casperdcl
casperdcl approved these changes Mar 25, 2022
View reviewed changes
Copy link
Contributor

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

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

:)

@casperdcl casperdcl merged commit 81872fa into master Mar 25, 2022
@casperdcl casperdcl deleted the fix-cic-cml-in-cml branch March 25, 2022 20:16
@dacbd dacbd mentioned this pull request Sep 9, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@casperdcl casperdcl casperdcl approved these changes

@dacbd dacbd dacbd approved these changes

Assignees

@0x2b3bfa0 0x2b3bfa0

Labels
bug Something isn't working resource-runner iterative_runner TF resource
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cml <sub-cmd> on cml runner instance fails
3 participants
@0x2b3bfa0 @dacbd @casperdcl