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

[DOCS] Update Microsoft SQL Server integration docs with required permissions to the database #5934

Merged
merged 2 commits into from Jun 1, 2023
Merged

[DOCS] Update Microsoft SQL Server integration docs with required permissions to the database #5934

merged 2 commits into from Jun 1, 2023

Conversation

geekpete
Copy link
Member

What does this PR do?

Add missing required information to the docs for Microsoft SQL Server integration.
This detail was already documented for the Metricbeat MSSQL Module docs and I've taken this detail directly from there.

Related issues

@geekpete geekpete requested a review from a team as a code owner April 20, 2023 05:03
@geekpete geekpete changed the title [DOCS] Update docs with required permissions to use MSSQL integration [DOCS] Update Microsoft SQL Server integration docs with required permissions to the database Apr 20, 2023
@geekpete geekpete added documentation Improvements or additions to documentation Team:Docs Label for the Observability docs team docs labels Apr 20, 2023
@geekpete
Copy link
Member Author

I think I probably also need to add an entry to the Changelog even for docs fixes?

@elasticmachine
Copy link

elasticmachine commented Apr 20, 2023 •
edited

💚 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-06-01T08:56:37.541+0000

  • Duration: 17 min 16 sec

Test stats 🧪

Test Results
Failed 0
Passed 19
Skipped 0
Total 19

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@geekpete
Copy link
Member Author

Let me know whatever I need to do to progress this one or feel free to perform any actions/chjanges as needed to get integration test to pass.

ritalwar
ritalwar reviewed Apr 26, 2023
View reviewed changes
Copy link
Contributor

@ritalwar ritalwar left a comment

Choose a reason for hiding this comment

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

Do you think that one reference is sufficient, or should we also consider providing msdn permission links for each table?

@ritalwar
Copy link
Contributor

I think I probably also need to add an entry to the Changelog even for docs fixes?

yes, it's needed.

@ritalwar
Copy link
Contributor

Let me know whatever I need to do to progress this one or feel free to perform any actions/chjanges as needed to get integration test to pass.

You must rebuild and commit.

@ritalwar ritalwar mentioned this pull request Apr 26, 2023
Closed
@geekpete
Copy link
Member Author

I'll look to include the MSDN link for each table and also rebuild.

@lalit-satapathy
Copy link
Collaborator

@ritalwar, Is it ready to be closed?

@elasticmachine
Copy link

elasticmachine commented May 23, 2023 •
edited

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (2/2) 💚
Files 100.0% (2/2) 💚
Classes 100.0% (2/2) 💚
Methods 93.75% (30/32) 👎 -2.917
Lines 100.0% (1251/1251) 💚
Conditionals 100.0% (0/0) 💚

@geekpete
Copy link
Member Author

Added links for each permission required, bumped version and also did a build so that checks now pass.
Let me know if there's anything else I need to tweak for this PR, or alternatively just make any edits as needed to get it through. Thanks

@ritalwar
Copy link
Contributor

Added links for each permission required

It appears that your changes to README.md were deleted in your last commit. Only the changelog.yml file has been modified as part of this PR. Could you please commit the changes to README.md as well?

@geekpete
Copy link
Member Author

geekpete commented May 23, 2023 •
edited

Sorry, fixing.
Rebased from upstream main and applied my changes fresh and bumped version to the next correct increment.
Hopefully this clears things up.

@ritalwar
Copy link
Contributor

@ritalwar, Is it ready to be closed?

once all the review comments have been addressed, this can be closed.

ritalwar
ritalwar approved these changes May 25, 2023
View reviewed changes
Copy link
Contributor

@ritalwar ritalwar left a comment

Choose a reason for hiding this comment

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

LGTM!

@geekpete geekpete requested a review from a team as a code owner June 1, 2023 07:59
@geekpete @ritalwar
Update packages/microsoft_sqlserver/changelog.yml
defde36 
Co-authored-by: Richa Talwar <102972658+ritalwar@users.noreply.github.com>
@geekpete geekpete merged commit 7954d74 into elastic:main Jun 1, 2023
3 checks passed
@elasticmachine
Copy link

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

sodhikirti07 pushed a commit that referenced this pull request Jun 15, 2023
@geekpete @sodhikirti07
[DOCS] Update Microsoft SQL Server integration docs with required per…
9de1c86 
…missions to the database (#5934)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ritalwar ritalwar ritalwar approved these changes

Assignees
No one assigned
Labels
docs documentation Improvements or additions to documentation Team:Docs Label for the Observability docs team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[MSSQL] Update documentation with permission link
4 participants
@geekpete @elasticmachine @ritalwar @lalit-satapathy