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.

Sign up for GitHub

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

Jump to bottom

[Salesforce] Add request timeout parameter in configuration #7891

Merged
merged 4 commits into from
Sep 21, 2023
Merged

[Salesforce] Add request timeout parameter in configuration #7891

merged 4 commits into from
Sep 21, 2023

Conversation

milan-elastic
Copy link
Contributor

@milan-elastic milan-elastic commented Sep 20, 2023 •
edited
Loading

  • Enhancement

What does this PR do?

  • This PR increases the default timeout 30s to 5m (by adding a new field) for the following datastreams: login-rest, logout-rest, setupaudittrail, apex-rest
  • If users by any chance still facing a timeout issue then they can increase the timeout from the UI advanced option to get unblocked from the timeout issue.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • Verified that increase timeout value to 10m/5m works with the mock server where the default 30s was not working.
  • Verify that the default value of request_timeout is 30 seconds by keeping the parameter empty.'
  • Verify that the data collection works as expected by default after the addition of request_timeout paramter without any errors.

Related issues

Screenshots

image

@milan-elastic milan-elastic self-assigned this Sep 20, 2023
@milan-elastic milan-elastic added the enhancement New feature or request label Sep 20, 2023
@elasticmachine
Copy link

elasticmachine commented Sep 20, 2023 •
edited
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-09-21T10:01:11.285+0000

  • Duration: 18 min 55 sec

Test stats 🧪

Test Results
Failed 0
Passed 34
Skipped 0
Total 34

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Sep 20, 2023 •
edited
Loading

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (6/6) 💚
Files 100.0% (6/6) 💚
Classes 100.0% (6/6) 💚
Methods 98.649% (73/74) 👍 3.587
Lines 96.281% (1372/1425) 👍 16.435
Conditionals 100.0% (0/0) 💚

@milan-elastic milan-elastic marked this pull request as ready for review September 20, 2023 14:00
@milan-elastic milan-elastic requested a review from a team as a code owner September 20, 2023 14:00
lalit-satapathy
lalit-satapathy reviewed Sep 21, 2023
View reviewed changes
Copy link
Collaborator

@lalit-satapathy lalit-satapathy left a comment

Choose a reason for hiding this comment

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

Can we verify once that the agent has no issue with this high timeout, when it has to wait for 5mins?

packages/salesforce/data_stream/login_rest/manifest.yml Outdated Show resolved Hide resolved
@milan-elastic
Copy link
Contributor Author

Can we verify once that the agent has no issue with this high timeout, when it has to wait for 5mins?

Yes, we have checked with 5min, there are no issues we noticed after setting high timeout.

lalit-satapathy
lalit-satapathy approved these changes Sep 21, 2023
View reviewed changes
Copy link
Collaborator

@lalit-satapathy lalit-satapathy left a comment

Choose a reason for hiding this comment

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

LGTM

shmsr
shmsr requested changes Sep 21, 2023
View reviewed changes
Copy link
Member

@shmsr shmsr left a comment

Choose a reason for hiding this comment

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

Left a few comments.

packages/salesforce/data_stream/apex/manifest.yml Outdated Show resolved Hide resolved
packages/salesforce/data_stream/login_rest/manifest.yml Outdated Show resolved Hide resolved
packages/salesforce/data_stream/login_rest/manifest.yml Outdated Show resolved Hide resolved
packages/salesforce/_dev/build/docs/README.md Outdated Show resolved Hide resolved
packages/salesforce/data_stream/apex/manifest.yml Outdated Show resolved Hide resolved
shmsr
shmsr reviewed Sep 21, 2023
View reviewed changes
Copy link
Member

@shmsr shmsr left a comment

Choose a reason for hiding this comment

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

Just a couple more nitpicks

packages/salesforce/docs/README.md Outdated Show resolved Hide resolved
packages/salesforce/_dev/build/docs/README.md Outdated Show resolved Hide resolved
@milan-elastic milan-elastic merged commit 64df4f8 into elastic:main Sep 21, 2023
4 checks passed
@elasticmachine
Copy link

Package salesforce - 0.10.1 containing this change is available at https://epr.elastic.co/search?package=salesforce

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shmsr shmsr shmsr approved these changes

@lalit-satapathy lalit-satapathy lalit-satapathy approved these changes

Assignees

@milan-elastic milan-elastic

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Salesforce] Add the Request timeout interval for salesforce integration
4 participants
@milan-elastic @elasticmachine @shmsr @lalit-satapathy