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

add support for Kind attribute of LogpushJob #936

Merged
merged 1 commit into from Jun 22, 2022

Conversation

sbfaulkner
Copy link
Contributor

@sbfaulkner sbfaulkner commented Jun 17, 2022

Warning based on Shopify:logpush-null-filter branch - see PR #937

Description

This PR is to address the addition of the kind attribute to the LogpushJob object.

(and as part of implementing tests I discovered and fixed a "small" json marshalling issue)

Has your change been tested?

Modified and extended existing tests to cover multiple cases for get and create (which addresses marshal and unmarshal).

Types of changes

What sort of change does your code introduce/modify?

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • This change is using publicly documented (api.cloudflare.com or developers.cloudflare.com) and stable APIs.

Note while not yet documented, my understanding is that it will be soon

cc: @yknx4

@sbfaulkner sbfaulkner changed the title add support for Kind attribut of LogpushJob add support for Kind attribute of LogpushJob Jun 17, 2022
@jacobbednarz
Copy link
Member

the marshaling issue being cloudflare/terraform-provider-cloudflare#1703?

depending on timelines for documenting that new attribute, you may want to split this and handle them separately. I can check with the team when I'm in front of a computer next.

@sbfaulkner
Copy link
Contributor Author

@jacobbednarz

the marshaling issue being cloudflare/terraform-provider-cloudflare#1703?

in my case I observed it marhalling a null filter as "filter": "{\"where\":{}}" -- and had to resolve that in order to test the changes I was making

so yes, I'll pull it into a separate issue and see if I can maybe resolve the other issue too

@sbfaulkner
Copy link
Contributor Author

@jacobbednarz updated to handle the null filter in #937

@sbfaulkner
Copy link
Contributor Author

@jacobbednarz

depending on timelines for documenting that new attribute, you may want to split this and handle them separately. I can check with the team when I'm in front of a computer next.

note: the attribute is present and supported now

@jacobbednarz
Copy link
Member

are you able to link me to where the docs are that you're referencing? the ones that i can see that include a kind attribute are actually for rulesets and the kind is already supported - https://github.com/cloudflare/cloudflare-go/blob/master/rulesets.go#L14-L18 (but is a different API to the one here)

@sbfaulkner
Copy link
Contributor Author

@jacobbednarz sorry,I didn't mean it was documented yet - I meant it was present and supported in the API

@jacobbednarz
Copy link
Member

gotcha, we'll need to wait for the public API documentation to land to add support here but it's fine to leave the PR pending that for now.

could you remove the other changes from #937 from here so it's a clear representation of what we're looking to merge?

@sbfaulkner
Copy link
Contributor Author

@jacobbednarz I've rebased, so it now contains only these changes - and I've reached out to the PM on your side to let them know that the docs are a blocker for the merge

@jacobbednarz
Copy link
Member

PCX will be doing a revamp of these docs but stability has been confirmed, so moving forward with this one.

@jacobbednarz jacobbednarz merged commit 7730055 into cloudflare:master Jun 22, 2022
@sbfaulkner sbfaulkner deleted the logpushjob-kind-edge branch June 23, 2022 15:38
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.

None yet

2 participants