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

Augment Channel for other possible fields #48

Closed
wants to merge 1 commit into from
Closed

Augment Channel for other possible fields #48

wants to merge 1 commit into from

Conversation

jakedavis
Copy link

Hi!

We've found ourselves really wanting the "Details" from the channel field within the first trigger log entry, and noticed that the struct in this library doesn't include it as a field. I'm hoping we can augment that struct so we can access it within our project. The commit contains a bit more context as well.

I couldn't find much documentation about channels, so I don't know if this will cause any bad effects, but it appears to me that these four fields are consistent.

Apologies for the little whitespace change; I can recommit without if need be. My go linting tool does that automatically.

Thanks for reviewing!

The Channel struct is one of the "includes" fields for log entries.
Beyond having a type, it can also have several other fields that
provide information about the incident.

The Details field is particularly useful, as the first log entry for
every incident includes arbitrary data that can be sent by the service
opening the incident.
@jakedavis
Copy link
Author

I managed to find some more information from the v1 API documentation, and it looks like I've configured it specifically for the Web Trigger type:

https://v1.developer.pagerduty.com/documentation/rest/log_entries/show

So it sounds like this PR won't work as-is, but maybe I can instead change Channel to be a *json.RawMessage instead?

@ranjib
Copy link
Contributor

ranjib commented Oct 24, 2016

@jakedavis that is exactly what i am thinking. We would be loosing the known "type" entry in this route, i tried this without success

type Channel struct{
    json.RawMessage
    Type string `json:"type"`
}

@atomicules
Copy link
Contributor

I'm newish to Go, but have had to tweak this to pull out a detail I'm interested in. I.e. I've ended up with:

// Channel is the means by which the action was carried out.
type Channel struct {
	Type string
	Details struct {
		Check struct {
			FieldIWant string `json:"field_I_want"`
		} `json:"check"`
	} `json:"details"`
}

Hard to see how that could be made flexible enough for everyone using structs and json. Maybe https://github.com/tidwall/gjson would be better... but I haven't investigated yet.

atomicules added a commit to atomicules/go-pagerduty that referenced this pull request Apr 2, 2018
Would be nice to have a flexible way to do this:

PagerDuty#48

But in the meantime...
dobs added a commit to dobs/go-pagerduty that referenced this pull request Mar 19, 2019
This branch incorporates a bunch of old un-merged PRs from upstream, specifically:

- PagerDuty#72
- PagerDuty#73
- PagerDuty#74
- PagerDuty#75
- PagerDuty#78
- PagerDuty#91
- PagerDuty#93
- PagerDuty#97
- PagerDuty#104
- PagerDuty#106
- PagerDuty#108
- PagerDuty#113
- PagerDuty#114
- PagerDuty#126
- PagerDuty#127
- PagerDuty#130
- PagerDuty#132
- PagerDuty#137
- PagerDuty#138
- PagerDuty#140
- PagerDuty#143

It doesn't attempt to merge, due to potential complexity:

- PagerDuty#48
- PagerDuty#99
- PagerDuty#134

And skips the following as it was picked up as part of pulling in the above merged PRs:

- PagerDuty#133

Will have subsequent PRs for cleanup, specifically:
- Reducing duplicate code.
- Standardizing response and error handling.

Followed by more janitorial and refactor-oriented changes.
@stmcallister
Copy link
Contributor

We completely agree this should be addressed, but as mentioned above not with this implementation. Going to close this now, but would welcome additional ideas going forward.

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.

4 participants