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

feat: in docker debug support #1789

Merged
merged 7 commits into from
Sep 30, 2021

Conversation

dadrus
Copy link
Contributor

@dadrus dadrus commented Sep 27, 2021

There is no related issue. The PR adds the possibility to debug Kratos in Docker. Since my entire local setup is dockerized, I was using this each time I was working on a PR for Kratos. Since I think I'm not the only one having such a setup, I hope the community will find this option useful. If this PR is going to be accepted, I'll extend the documentation on how to make use of it.

Checklist

  • I have read the contributing guidelines.
  • I am following the contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security. vulnerability, I confirm that I got green light (please contact security@ory.sh) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works. (No tests possible)
  • I have added or changed the documentation.

@aeneasr
Copy link
Member

aeneasr commented Sep 27, 2021

Thank you! Maybe it makes sense to move this to contrib? On a side note, we usually delimit docker images with - not . :)

@dadrus
Copy link
Contributor Author

dadrus commented Sep 27, 2021

You're welcome. :) Do you mean to move the corresponding code to contrib or to add the corresponding documentation there? Actually, I don't mind. I do however think the places where I put the two files are better suited for the purpose as these are easier to find for the people.
As for the documentation itself I was thinking about the /docs/docs/debug directory, but if you have a better place in mind, please let me know.

@codecov
Copy link

codecov bot commented Sep 27, 2021

Codecov Report

Merging #1789 (26b8c44) into master (9c365ea) will not change coverage.
The diff coverage is n/a.

❗ Current head 26b8c44 differs from pull request most recent head 6bd3d29. Consider uploading reports for the commit 6bd3d29 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1789   +/-   ##
=======================================
  Coverage   74.11%   74.11%           
=======================================
  Files         260      260           
  Lines       12770    12770           
=======================================
  Hits         9465     9465           
  Misses       2674     2674           
  Partials      631      631           

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 9c365ea...6bd3d29. Read the comment docs.

@dadrus
Copy link
Contributor Author

dadrus commented Sep 28, 2021

@aeneasr : I've added documentation as well as a template file for docker-compose based setups. Please take a look.
Where can I reference the new document from, or is it automatically included?

Copy link
Member

@aeneasr aeneasr 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! The last change would be adding the docs to the sidebar :)

https://github.com/ory/kratos/blob/master/docs/sidebar.json

@@ -0,0 +1,138 @@
# Debugging Kratos in Docker
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Debugging Kratos in Docker
---
id: debug-docker-delve-ory-kratos
title: Debugging Ory Kratos in Docker with Delve
---

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@aeneasr aeneasr merged commit 37325a1 into ory:master Sep 30, 2021
@aeneasr
Copy link
Member

aeneasr commented Sep 30, 2021

🙏

@dadrus dadrus deleted the feature/in_docker_debug_support branch September 30, 2021 11:02
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