db_job_reset requires schedule, db_job_update doesn't allow updating of parameters

[nick-treece]

I'm just testing out replacing a workflow that previously used the Databricks R SDK with brickster.

I've noticed is that db_jobs_reset has schedule as a mandatory argument despite neither db_jobs_create or db_jobs_update requiring a schedule. It would be good to have this as a optional argument to keep it consistent with the other functions, and because a schedule may not always be required.

In addition db_jobs_update doesn't allow you to pass a parameters argument, which may feasibly want to be updated if other things about the job could be updated. Could that be added?

[zacdav-db]

Thanks for raising this, can you please test the changes in #132 before I merge.

[nick-treece]

Hi Zac,

I've done some testing on this today, but it appears that the 'enabled' field is also required for db_jobs_reset, but neither db_jobs_update or db_jobs_create. It would be good to have alignment across all these functions as to which are required arguments and which are optional (with a preference of most being optional).

In addition I tried the db_jobs_update function and attempted to remove a parameter from the job definition however the job is still containing that parameter when looking in the UI. I did test adding a parameter and that did appear to work, however I would expect that the parameters would be overwritten wholly and any that aren't in the list passed to db_jobs_update(parameters = list()) would be removed.

Happy to discuss in further detail.

[zacdav-db]

I've done some testing on this today, but it appears that the 'enabled' field is also required for db_jobs_reset, but neither db_jobs_update or db_jobs_create

Thanks, a repro/mock example would be great as I'm not clear on the field. Then I can investigate.

In addition I tried the db_jobs_update function and attempted to remove a parameter from the job definition however the job is still containing that parameter when looking in the UI. I did test adding a parameter and that did appear to work, however I would expect that the parameters would be overwritten wholly and any that aren't in the list passed to db_jobs_update(parameters = list()) would be removed.

This is because the update endpoint is for partial updates, you need to be explicit for each item.

The same behaviour is true for the python SDK:

from databricks.sdk import WorkspaceClient
from databricks.sdk.service.jobs import JobSettings, JobParameterDefinition

w = WorkspaceClient()

# job already has parameter 'a' with default 'b'
# this will not remove the parameter
w.jobs.update(
    job_id='<job-id>',
    new_settings=JobSettings(
        parameters=[JobParameterDefinition('x', 'y')]
    )
)

To make this work in python you need to do the following:

# must set `fields_to_remove` at same time
w.jobs.update(
    job_id='<job-id>',
    new_settings=JobSettings(
        parameters=[JobParameterDefinition('x', 'y')]
    ),
    fields_to_remove=["parameters"]
)

For which the corresponding code in brickster is:

library(brickster)

db_jobs_update(
  job_id = "<job-id>",
  fields_to_remove = "parameters",
  parameters = list(x = "y")
)

This is expected behaviour. The question to if its "intuitive" is a different one, but I'm inclined to try make this clearer in docs first rather then deviate from the behaviour thats consistent with the other SDKs.

[nick-treece]

Hey Zac,

I've put together a minimal repo that demonstrates the issue with db_jobs_reset here:
https://github.com/nick-treece/brickster_job_reset

[zacdav-db]

Thanks @nick-treece, this seems to work for me as-is when I supply my own token and change the existing_cluster_id.

What do you observe?

[nick-treece]

I get the following error message:

Error in httr2::req_perform(httr2::req_error(req, body = db_req_error_body)) :
HTTP 400 Bad Request.
ℹ INVALID_PARAMETER_VALUE: Required field 'enabled' is missing in queue
settings
Error in httr2::req_perform():

[zacdav-db]

Can you run sessionInfo() and provide the output here please?

And what runtime is the cluster that you're running this on?

Update: I've tried a few times to repro now but unfortunately have been unsuccessful 😵, even when trying different runtimes and branches.

[nick-treece]

DBR 15.4

sessionInfo() output

R version 4.3.2 (2023-10-31)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 22.04.5 LTS

Matrix products: default
BLAS/LAPACK: /opt/OpenBLAS/lib/libopenblasp-r0.3.20.so; LAPACK version 3.9.0

locale:
[1] LC_CTYPE=C.UTF-8 LC_NUMERIC=C LC_TIME=C.UTF-8
[4] LC_COLLATE=C.UTF-8 LC_MONETARY=C.UTF-8 LC_MESSAGES=C.UTF-8
[7] LC_PAPER=C.UTF-8 LC_NAME=C LC_ADDRESS=C
[10] LC_TELEPHONE=C LC_MEASUREMENT=C.UTF-8 LC_IDENTIFICATION=C

time zone: Etc/UTC
tzcode source: system (glibc)

attached base packages:
[1] stats graphics grDevices utils datasets methods base

other attached packages:
[1] brickster_0.2.10

loaded via a namespace (and not attached):
[1] vctrs_0.6.5 cli_3.6.5 rlang_1.1.6 DBI_1.2.3
[5] processx_3.8.6 purrr_1.0.2 pkgload_1.4.1 generics_0.1.4
[9] jsonlite_2.0.0 SparkR_3.5.0 glue_1.7.0 dbplyr_2.5.1
[13] pkgbuild_1.4.8 ps_1.9.1 rappdirs_0.3.3 tibble_3.3.0
[17] ellipsis_0.3.2 fastmap_1.1.1 lifecycle_1.0.4 httr2_1.2.1
[21] Rserve_1.8-13 memoise_2.0.1 compiler_4.3.2 dplyr_1.1.4
[25] fs_1.6.6 sessioninfo_1.2.3 pkgconfig_2.0.3 R6_2.6.1
[29] tidyselect_1.2.1 pillar_1.11.1 usethis_3.2.1 curl_7.0.0
[33] callr_3.7.6 magrittr_2.0.3 tools_4.3.2 devtools_2.4.6
[37] remotes_2.5.0 cachem_1.0.8 desc_1.4.3

[nick-treece]

I just tried this with DBR 17.3 to see if that resolved the issue but I observe the same behaviour I originally reported.

[zacdav-db]

I strongly suspect something is different at the workspace level for some reason 🤔.

I'll try it on azure databricks since I can see thats the obvious difference currently.

[zacdav-db]

Okay, I was able to repro it in Azure Databricks 😵