Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Monitor data source #92

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

Conversation

evansmurithi
Copy link

Fixes #91

@evansmurithi
Copy link
Author

@louy could you please have a look at this PR?

@louy
Copy link
Owner

louy commented Oct 27, 2020

Any chance we could add tests for this?

@evansmurithi
Copy link
Author

sure, on it

@evansmurithi evansmurithi marked this pull request as draft October 28, 2020 07:56
@evansmurithi evansmurithi force-pushed the add-uptimerobot-monitor-data-source branch from 2cc63cb to e8b0bb3 Compare October 28, 2020 20:59
@evansmurithi evansmurithi marked this pull request as ready for review October 28, 2020 20:59
@evansmurithi
Copy link
Author

@louy it's ready for review now

uptimerobot/api/monitor.go Outdated Show resolved Hide resolved
}

func dataSourceMonitorRead(d *schema.ResourceData, m interface{}) error {
monitors, err := m.(uptimerobotapi.UptimeRobotApiClient).GetMonitors()
Copy link
Owner

Choose a reason for hiding this comment

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

Is this the most efficient way to get a single monitor? Why not iterate over the list checking IDs only, and the build the Monitor object only for that single monitor we're interested in?

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense. Refactored to return monitor IDs that match the provided friendly_name

@evansmurithi evansmurithi force-pushed the add-uptimerobot-monitor-data-source branch from e8b0bb3 to 0a1d497 Compare October 29, 2020 14:32
@louy
Copy link
Owner

louy commented Oct 29, 2020

So I'm just wondering, would it make sense to use the search parameter in UR's api method getMonitors?

@evansmurithi
Copy link
Author

Yeah, that would actually return few records to sift through

@evansmurithi
Copy link
Author

@louy updated to use the search parameter

@evansmurithi
Copy link
Author

@louy should be ready for review

Compare page offset to total monitor records to know when to break from
loop. This will avoid infinit loops in situations where a monitor gets
added/removed mid-call.
Avoid constructing Monitor struct for each and every monitor record.
Return IDs of monitor records that match the friendly name provided and
build the Monitor object for the single monitor found.
Leverage the search parameter in getMonitors API so as to get filtered
results instead of getting all monitors
@evansmurithi evansmurithi force-pushed the add-uptimerobot-monitor-data-source branch from c4fc6ef to 131f2dc Compare November 26, 2020 17:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support data source for monitors
2 participants