-
Notifications
You must be signed in to change notification settings - Fork 1
DACKAR Architecture Design #21
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
Conversation
…wangc/architecture
mandd
left a comment
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.
this in my initial set of comments, i'd like to give a second pass to this MR and meet with you again to talk about about this MR
| logger.addHandler(fh) | ||
|
|
||
| def main(): | ||
| logger.info('Welcome to use DACKAR!') |
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.
suggestion:
print(""":::::::-. :::. .,-::::: ::: . :::. :::::::..
;;, ';, ;;;; ,;;;'````' ;;; .;;,. ;;;; ;;;;``;;;; [[ [[ ,[[ '[[, [[[ [[[[[/' ,[[ '[[, [[[,/[[['
$$,
888,o8P' 888 888,`88bo,__,o,"888"88o, 888 888,888b "88bo,
MMMMP"` YMM ""` "YUMMMMMP"MMM "MMP"YMM ""` MMMM "W" '
""")
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.
Sorry, what do you mean here?
|
|
||
|
|
||
| class WorkOrderProcessing(WorkflowBase): | ||
| """ |
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.
we talked about renaming this pipeline
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.
same applies to operator shift logs
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.
Addressed in PR #22
| [neo4j] | ||
| uri = "neo4j://localhost:7687" | ||
| pwd = "123456789" | ||
| config_file_path = "/Users/wangc/Library/Application Support/Neo4j Desktop/Application/relate-data/dbmss/dbms-28cea4a5-ad08-4fed-94a1-70d44e297586/conf/neo4j.conf" |
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.
should we remove this personal information?
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.
addressed in PR #22
|
|
||
|
|
||
| class WorkOrderProcessing(WorkflowBase): | ||
| """ |
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.
same applies to operator shift logs
|
|
||
| class WorkOrderProcessing(WorkflowBase): | ||
| """ | ||
| Class to process CWS work order dataset |
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.
remove CWS reference
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.
removed
| [neo4j] | ||
| uri = "neo4j://localhost:7687" | ||
| pwd = "123456789" | ||
| # User need to set the import folder, and all files for nodes and edges are using relative path to the import folder |
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.
should we anonymize these lines?
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.
addressed in PR #22
|
@mandd FYI, I have addressed your comments in a different PR, i.e., PR#22 |
|
Test board is green. MR is good on my end. Thanks! |
Pull Request Description
What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)
What are the significant changes in functionality due to this change request?
For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.