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

docs: updated brackets in OAuth Authentication #1798

Merged

Conversation

davidnateberg
Copy link
Contributor

@davidnateberg davidnateberg commented Jan 31, 2022

Description

I changed what I believe to be incorrect brackets in the Security docs for the OAuth method

During a deployment of Apache Airflow I copied a chunk of code from the OAuth method in the Security documentation. When deploying I noticed errors related to the brackets and I believe that the bracket placement in the documentation is incorrect. I understand it's supposed to be something you copy what you need out of but if the whole chunk is copied it can be confusing why it doesnt work.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Is CRUD MVC related.
  • Is Auth, RBAC security related.
  • Changes the security db schema.
  • Introduces new feature
  • Removes existing feature

@davidnateberg davidnateberg changed the title updated brackets in OAuth Authentication docs: updated brackets in OAuth Authentication Jan 31, 2022
@codecov
Copy link

codecov bot commented Jan 31, 2022

Codecov Report

Merging #1798 (5ec037c) into master (f6f66fc) will increase coverage by 0.04%.
The diff coverage is n/a.

❗ Current head 5ec037c differs from pull request most recent head b597de7. Consider uploading reports for the commit b597de7 to get more accurate results

@@            Coverage Diff             @@
##           master    #1798      +/-   ##
==========================================
+ Coverage   77.08%   77.12%   +0.04%     
==========================================
  Files          56       56              
  Lines        8226     8232       +6     
==========================================
+ Hits         6341     6349       +8     
+ Misses       1885     1883       -2     
Flag Coverage Δ
python 77.12% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
flask_appbuilder/security/decorators.py 93.33% <0.00%> (-0.57%) ⬇️
flask_appbuilder/security/manager.py 75.88% <0.00%> (+0.26%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f96c4f...b597de7. Read the comment docs.

Copy link
Owner

@dpgaspar dpgaspar left a comment

Choose a reason for hiding this comment

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

Thank you for fixing, left a small comment

docs/security.rst Outdated Show resolved Hide resolved
@davidnateberg
Copy link
Contributor Author

@dpgaspar the brackets are fixed and this can be merged now

Copy link
Owner

@dpgaspar dpgaspar left a comment

Choose a reason for hiding this comment

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

Thank you, left a comment

docs/security.rst Outdated Show resolved Hide resolved
@davidnateberg
Copy link
Contributor Author

Don't want to pester @dpgaspar but this is still open, was there anything else you wanted changed? I ended up just copying your block in the end

@dpgaspar dpgaspar merged commit 06664bd into dpgaspar:master Apr 8, 2022
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