New Resource: `azurerm_scheduler_job` #1172

katbyte · 2018-04-27T02:15:33Z

No description provided.

travis CI fixes

tombuildsstuff

Apologies for the delayed review of this PR - I've taken a look through and left some comments in-line; but I've got questions both about the schema design (I think we should make this more Terraform-y) and how the tests are implemented (to make them easier to read)

Thanks!

tombuildsstuff · 2018-05-14T21:14:25Z

azurerm/helpers/supress/suppress.go

@@ -0,0 +1,23 @@
+package supress


supress -> suppress

tombuildsstuff · 2018-05-14T21:14:31Z

azurerm/helpers/supress/suppress.go

+	"time"
+
+	"github.com/hashicorp/terraform/helper/schema"
+	"strings"


minor can we sort these?

tombuildsstuff · 2018-05-14T21:27:48Z

azurerm/resource_arm_scheduler_job.go

+								Schema: map[string]*schema.Schema{
+									"day": { // DatOfWeek (sunday monday)
+										Type:     schema.TypeString,
+										Required: true,


can we add some validation here?

tombuildsstuff · 2018-05-24T10:54:12Z

azurerm/helpers/supress/suppress_test.go

+
+	for _, tc := range cases {
+		if CaseDifference("test", tc.StringA, tc.StringB, nil) != tc.Suppress {
+			t.Fatalf("Expected CaseDifference to return %t for '%s' == '%s'", tc.Suppress, tc.StringA, tc.StringB)


minor you can use %q in place of '%s' (or "%s")

Should we be using %q everywhere instead of %s?

tombuildsstuff · 2018-05-24T10:55:17Z

azurerm/helpers/validate/validate.go

+	url, err := url.Parse(v)
+	if err != nil {
+		errors = append(errors, fmt.Errorf("%q url is in an invalid format: %q (%+v)", k, i, err))
+	} else if url.Scheme != "http" && url.Scheme != "https" {


that's a bit of an assumption given the name of the function - perhaps it's worth renaming this to WebUrl?

tombuildsstuff · 2018-05-25T11:20:01Z

azurerm/resource_arm_scheduler_job_test.go

+    method = "put"
+    body   = "this is some text"
+    headers = {
+      Content-Type = "text"


we should quote this, since I think this could break in HCL2?

tombuildsstuff · 2018-05-25T11:20:30Z

azurerm/resource_arm_scheduler_job_test.go

+      tenant_id = "%s"
+      client_id = "%s"
+      secret    = "%s"
+      audience  = "https://management.core.windows.net/"


can we template this value? it's available on meta.(*ArmClient).environment..

tombuildsstuff · 2018-05-25T11:20:46Z

azurerm/resource_arm_scheduler_job_test.go

+		resource.TestCheckResourceAttr(resourceName, "action_web.0.authentication_active_directory.0.tenant_id", tenantId),
+		resource.TestCheckResourceAttr(resourceName, "action_web.0.authentication_active_directory.0.client_id", clientId),
+		resource.TestCheckResourceAttr(resourceName, "action_web.0.authentication_active_directory.0.secret", secret),
+		resource.TestCheckResourceAttr(resourceName, "action_web.0.authentication_active_directory.0.audience", "https://management.core.windows.net/"),


we can either use the full URI depending on the environment being used here or just ensure this is set?

tombuildsstuff · 2018-05-25T11:21:18Z

examples/scheduler-jobs/main.tf

+    body   = "The job failed"
+
+    headers = {
+      Content-Type = "text"


tombuildsstuff · 2018-05-25T11:21:46Z

azurerm/resource_arm_scheduler_job.go

+	return hashcode.String(buf.String())
+}
+
+//todo where to put these?


azurerm/helpers/set/set.go?

tombuildsstuff

Aside from a few minor nits (which I'll push a commit for) this otherwise LGTM 👍

tombuildsstuff · 2018-06-19T11:12:41Z

azurerm/helpers/validate/validate.go

+		}
+
+		for _, s := range validSchemes {
+			if url.Scheme == s {


IMO this should be strings.EqualFold(url.Scheme, s) to handle casing - but it's not a blocker

tombuildsstuff · 2018-06-19T11:13:19Z

azurerm/helpers/validate/validate.go

+			}
+		}
+
+		return


should we be adding a couldn't find it error here?

tombuildsstuff · 2018-07-03T06:08:02Z

azurerm/helpers/validate/validate_test.go

@@ -0,0 +1 @@
+package validate


can we add some basic tests covering the methods in the other file?

tombuildsstuff · 2018-07-03T11:46:29Z

website/azurerm.erb

@@ -736,6 +736,11 @@
                  <a href="/docs/providers/azurerm/r/scheduler_job_collection.html">azurerm_scheduler_job_collection</a>
                </li>
              </ul>
+              <ul class="nav nav-visible">
+                <li<%= sidebar_current("docs-azurerm-resource-scheduler_job") %>>


we should suffix this to avoid highlight conflicts

tombuildsstuff · 2018-07-03T11:47:23Z

website/docs/r/scheduler_job.html.markdown

+Scheduler Job can be imported using a `resource id`, e.g.
+
+```shell
+terraform import azurerm_search_service.service1 /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resourceGroup1/providers/Microsoft.Scheduler/jobCollections/jobCollection1/jobs/job1


azurerm_search_service.service1 -> azurerm_scheduler_job.job1

tombuildsstuff · 2018-07-03T11:49:55Z

azurerm/resource_arm_scheduler_job_test.go

+		Steps: []resource.TestStep{
+			{
+				Config: testAccAzureRMSchedulerJob_web_basic(ri, testLocation()),
+				Check:  checkAccAzureRMSchedulerJob_web_basic(resourceName),


we should in-line these

tombuildsstuff · 2018-07-03T12:28:45Z

Tests pass:

ghost · 2020-03-30T15:25:50Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

katbyte added 2 commits April 26, 2018 18:55

Intial commit of scheduler_job

f1c9ef6

Merged master into f-r-scheduler_job

020df2c

katbyte added new-resource service/scheduler labels Apr 27, 2018

katbyte added this to the 1.5.0 milestone Apr 27, 2018

katbyte requested a review from tombuildsstuff April 27, 2018 02:15

katbyte force-pushed the f-r-scheduler_job branch 2 times, most recently from 7a07134 to 634f9b4 Compare April 27, 2018 15:56

azurerm_scheduler_job todo & misc cleanup

beb79b8

katbyte force-pushed the f-r-scheduler_job branch from 634f9b4 to beb79b8 Compare April 27, 2018 16:03

travis CI fixes

2c9a46a

travis CI fixes

katbyte force-pushed the f-r-scheduler_job branch from d5c9817 to 2c9a46a Compare April 28, 2018 20:59

tombuildsstuff modified the milestones: 1.5.0, 1.6.0 May 8, 2018

azurerm_scheduler_job: removed populate function & other small changes

720eabc

katbyte force-pushed the f-r-scheduler_job branch from 6b69b51 to 720eabc Compare May 13, 2018 03:22

katbyte added 5 commits May 16, 2018 16:23

aszurerm_schduler_job: update tests & validation

6b7b924

Updated example URLs

61ea612

Merge branch 'master' into f-r-scheduler_job

9a76a4b

schduler jobs & collection: formatted examples

ee60252

azurerm_scheduler_job: schema adjustments

218376a

tombuildsstuff self-assigned this May 23, 2018

tombuildsstuff modified the milestones: 1.6.0, 1.7.0 May 24, 2018

tombuildsstuff changed the title ~~Added new resource scheduler_job~~ New Resource: azurerm_scheduler_job May 25, 2018

tombuildsstuff suggested changes May 25, 2018

View reviewed changes

tombuildsstuff added the waiting-response label May 25, 2018

katbyte added 3 commits May 25, 2018 12:33

azurerm_scheduler_job: resolving PR comments

5a50522

azurerm_scheduler_job: First round of PR fixes

878707c

azurerm_scheduler_job: Futher PR fixes

e404a92

katbyte modified the milestones: 1.7.0, Soon Jun 12, 2018

tombuildsstuff modified the milestones: Soon, 1.8.0 Jun 19, 2018

katbyte added 3 commits June 21, 2018 11:41

Merge branch 'master' into f-r-scheduler_job

93e318b

scheduler job: address PR comments

c18fbfe

scheduler job - minor fix to tests

9836428

katbyte modified the milestones: 1.8.0, 1.9.0 Jun 28, 2018

Adding tests for the validation functions

770beb3

tombuildsstuff removed their assignment Jul 3, 2018

tombuildsstuff removed the waiting-response label Jul 3, 2018

tombuildsstuff added 2 commits July 3, 2018 13:50

inline-ing the check methods

f32ecc0

Fixing the highlight in the sidebar

184a879

tombuildsstuff approved these changes Jul 3, 2018

View reviewed changes

tombuildsstuff and others added 3 commits July 3, 2018 13:52

Merge branch 'master' into f-r-scheduler_job

00cfa00

Fixing merge issues

c1ae37e

Fixing a crash when loading the environment

f0a0315

tombuildsstuff merged commit 3cc5cdb into master Jul 3, 2018

tombuildsstuff deleted the f-r-scheduler_job branch July 3, 2018 12:29

tombuildsstuff added a commit that referenced this pull request Jul 3, 2018

Updating to include #1172

9dec4dd

tombuildsstuff added a commit that referenced this pull request Jul 3, 2018

Updating to include #1172

7c90c01

katbyte mentioned this pull request Jul 9, 2018

scheduler_job: Added storage queue support #1191

Merged

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_scheduler_job` #1172

New Resource: `azurerm_scheduler_job` #1172

katbyte commented Apr 27, 2018

tombuildsstuff left a comment

tombuildsstuff May 14, 2018

tombuildsstuff May 14, 2018

tombuildsstuff May 14, 2018

tombuildsstuff May 24, 2018

katbyte May 25, 2018

tombuildsstuff May 24, 2018

tombuildsstuff May 25, 2018

tombuildsstuff May 25, 2018

tombuildsstuff May 25, 2018

tombuildsstuff May 25, 2018

tombuildsstuff May 25, 2018

tombuildsstuff left a comment

tombuildsstuff Jun 19, 2018

tombuildsstuff Jun 19, 2018

tombuildsstuff Jul 3, 2018

tombuildsstuff Jul 3, 2018

tombuildsstuff Jul 3, 2018

tombuildsstuff Jul 3, 2018

tombuildsstuff commented Jul 3, 2018

ghost commented Mar 30, 2020

+              			}
+              		}
+              		return

New Resource: azurerm_scheduler_job #1172

New Resource: azurerm_scheduler_job #1172

Conversation

katbyte commented Apr 27, 2018

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff commented Jul 3, 2018

ghost commented Mar 30, 2020

New Resource: `azurerm_scheduler_job` #1172

New Resource: `azurerm_scheduler_job` #1172