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

added cr_buildtrigger_build #186

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

muschellij2
Copy link
Contributor

Point of this PR:

  1. Add cr_buildtrigger_build, which alllows us to construct buildtrigger objects. This is crucial when running the same code, but you don't want to replace a buildtrigger but it already exists. If you delete a buildtrigger, then any builds that were triggered from the previous trigger are not indicated to "Unknown". Now you can use cr_buildtrigger_build(...) and compare it to cr_buildtrigger_get() to see if the exact (or exact enough) buildtrigger already exists.
  2. Created the extract_trigger function for tidier code
  3. Keeping gcs_sources if they exist in the build
  4. Allow you to pass a build YAML argument

@muschellij2
Copy link
Contributor Author

muschellij2 commented Mar 1, 2022

Some example code of checking the same trigger where buildtrigger is the output of cr_buildtrigger_build

same_trigger = function(buildtrigger, name, project) {
  suppressMessages({
    res = try({
      googleCloudRunner::cr_buildtrigger_get(
        triggerId = name,
        projectId = project)
    }, silent = TRUE)
  })
  if (!inherits(res, "try-error") && !is.null(res)) {
    bt = googleCloudRunner:::as.buildTriggerResponse(buildtrigger)
    res$id = res$createTime = NULL
    if (is.null(res$disabled)) {
      res$disabled = FALSE
    }
    # do this if you don't care about description
    # !! Important - if you do care and you use a timestamped description - they are always different!
    res$description = bt$description = NULL
    all_names = union(names(bt), names(res))
    if (all(all_names %in% names(bt)) &&
        all(all_names %in% names(res))) {
      bt = bt[all_names]
      res = res[all_names]
    }

    if (isTRUE(all.equal(res, bt))) {
      message("trigger: ", name, " already exists and same as ",
              "the one being created, \nreturning the current trigger ",
              "instead of overwriting")
      return(TRUE)
    }
  }
  return(FALSE)
}

@MarkEdmondson1234
Copy link
Owner

MarkEdmondson1234 commented Mar 2, 2022

Add cr_buildtrigger_build, which alllows us to construct buildtrigger objects. This is crucial when running the same code, but you don't want to replace a buildtrigger but it already exists. If you delete a buildtrigger, then any builds that were triggered from the previous trigger are not indicated to "Unknown". Now you can use cr_buildtrigger_build(...) and compare it to cr_buildtrigger_get() to see if the exact (or exact enough) buildtrigger already exists.

I think this use case is better served via cr_buildtrigger_get() now returning NULL in 0.5.0 and using cr_buildtrigger_edit() to change existing triggers

# existing trigger?
present <- cr_buildtrigger_get("targets-scheduled-demo")

# not there
if(is.null(present)){
 # create trigger via cr_buildtrigger()
 trig <- cr_buildtrigger(...)
  return(trig)
} 

#already exisit - modify
cr_buildtrigger_edit(...)

But let me know what the above doesn't cover.

Keeping gcs_sources if they exist in the build

This broke builds in the past, does the existing change pass tests?

If you can't run the tests, it may help a lot if you have the ability to run them - the cloudbuild file for them are in inst/cloudbuild/cloudbuild_packages.yaml - to set them up you will need a googleCloudRunner enabled JSON auth key uploaded to your secret manager in the same project called "googlecloudrunner-test-key" then set up a trigger for your local fork.

Allow you to pass a build YAML argument

I don't see this change? Was that another pull?

@muschellij2
Copy link
Contributor Author

Agreed - no YAML argument.

Do you know what happens when a trigger is edited if it stays tied to specific builds?

@MarkEdmondson1234
Copy link
Owner

Yes, it's like editing them in the UI - the underlying triggerId remains intact, which is not the case if the buildtrigger is overwritten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants