-
Notifications
You must be signed in to change notification settings - Fork 57
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
feat: s3 data forwarding #636
base: master
Are you sure you want to change the base?
Conversation
@vsinghal13 can you take a look? |
@sumovishal this is not owned by Data Collection team. |
@ksbagr seems to be last person who made some changes to partitions. can you review this? |
Hi folks, any chance to get this in? |
Hey folks, any chance to get this in or do we have to maintain a fork of this from now on? |
@AyanGhatak can you take a look? |
sumologic/sumologic_partition.go
Outdated
func (s *Client) ListPartitions() ([]Partition, error) { | ||
var listPartitionResp ListPartitionResp | ||
|
||
data, _, err := s.Get("v1/partitions") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of fetching default no of partitions, it might be better to fetch max limit allowed 1000 to reduce number of api calls https://api.sumologic.com/docs/#operation/listPartitions
d.SetId(createdDfd.IndexID) | ||
} | ||
|
||
return resourceSumologicS3DataForwardingRuleUpdate(d, meta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be RuleRead not update
|
||
type S3DataForwardingDestination struct { | ||
ID string `json:"id,omitempty"` | ||
Name string `json:"destinationName,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please rename this to DestinationName for clarity
State: schema.ImportStatePassthrough, | ||
}, | ||
Schema: map[string]*schema.Schema{ | ||
"name": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here to rename this to destination_name to have similar naming convention with api
d.SetId(createdDfd.ID) | ||
} | ||
|
||
return resourceSumologicS3DataForwardingDestinationUpdate(d, meta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Read not Update
.github/workflows/release.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be changed, right @sumovishal
Tests are missing |
@ssharma-sumologic please review again. If the PR looks good, can you open it against the provider repo? We can't run acceptance tests here. |
|
||
func resourceSumologicS3DataForwardingRuleDelete(d *schema.ResourceData, meta interface{}) error { | ||
c := meta.(*Client) | ||
return c.DeleteS3DataForwardingRule(d.Id()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change this to d.index_id
return fmt.Errorf("S3DataForwardingRule for id %s not found", id) | ||
} | ||
|
||
d.SetId(dfr.IndexID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The id of a data forwarding rule isn't equal to the indexId. Please set the id of the rule here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe there's some confusion around Id and indexId. IndexId is the id of the partition/view , while the id is a unique identifier of the rule. Please see the docs https://api.sumologic.com/docs/#operation/createDataForwardingRule for more reference
@imranismail - We will be releasing the changes here. |
Support use-case to deploy S3 data forwarding destination and rules