-
Notifications
You must be signed in to change notification settings - Fork 16
Docs on new signal deployment details #1986
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
830388a
deployment + staging details
minhkhul 57cb4ec
wrap lines
nmdefries 6a289fd
Update _template_python/INDICATOR_DEV_GUIDE.md
minhkhul d2fafd1
Update _template_python/INDICATOR_DEV_GUIDE.md
minhkhul 67152f7
Update _template_python/INDICATOR_DEV_GUIDE.md
minhkhul 519582a
Update INDICATOR_DEV_GUIDE.md with mysql syntax
minhkhul 9972728
Update INDICATOR_DEV_GUIDE.md reorder instruction on cron jobs
minhkhul 01c9ebe
Merge branch 'main' into deployment-details
nmdefries File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
hostname publicly available here and here. User derivable 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.
@korlaxxalrok which of these should be removed? Do we need to change any user or host names?
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.
Yeah, those are pretty well exposed already, so no point in making changes for them.
Generally, my instinct is that this kind of documentation should live privately "elsewhere" and we should link to it from our public assets. I realize the "elsewhere" part of this has never been solved, or when attempted to be solved has not been consistently adopted, but it is something we should still consider for the future and treat like a best practice when possible.
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.
It doesnt hurt to scrub as much of the sensitive stuff as possible... Even if some bits of it are already "out there", reducing the number of current instances shrinks the attack surface (kinda) and makes it a little harder for a malicious actor to find useful information.
I definitely prefer having documentation like this in version control -- its easier to find, use, and see the timeline of changes. It puts instructions and details "next to" the code assets that are being discussed, and in a format closer to whats practical for developers and operations (as opposed to the complicated wysiwyg editor of gdocs).
We could create a new private documentation-only repository or even just a top-level
/documentationdirectory in the existing private https://github.com/cmu-delphi/delphi-admin repo. It wouldnt be as nice as having the docs in the same repo as the code, but it would be protected and cross-references will be handled well by the github web ui (for privileged users).Uh oh!
There was an error while loading. Please reload this page.
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.
ie:
(^ the issue title is shown for Delphi org members but is only shown as the bare url for a user without authN (and presumably un-authZ'd users too))
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.
Something like Docsify might be nice, for eventually building a docs site from Markdown files. I'd maybe push for a separate repo, to make it simpler to clone the repo without getting extra stuff (delph-admin comes with a lot of extra bits), to add access control, and to eventually make it easier to add CI to build a site from it.
We tried something like this before, but with a different tool (mkdocs), ultimately building it into a small site at https://delphi.cmu.edu/systems-handbook. There is a process to clone the repo, make changes, build it locally for review, and publish changes. It never caught on though, and the wheel kept going round.
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 agree completely, great points. The "where" is definitely a problem. I wanted to put the "how to make an indicator" manual in this repo so it's close to all the indicator code.
Alternative ideas:
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.
@melange396 opened an issue for this.
delphi-adminis private and so a good place to store sensitive info.