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

connection/jobs table update: track server connid, add user/id to jobs table #308

Merged
merged 13 commits into from
Nov 27, 2020

Conversation

ixcat
Copy link
Contributor

@ixcat ixcat commented Oct 30, 2020

to fix issue #87, #275 (add user/server connection to jobs table).

ideally, connection ID would be directly available via MyM; for the moment,
hack in a query to retrieve connection ID on connection and use this for
jobs table logging.

supersedes: #276

addresses: #276 (comment):

  • r.e. recurse: adjust to direct mym calls
  • r.e. indexing: feature too new, save ref and index that

status/remaining:

  • base functionality
  • unit test

guzman-raphael and others added 7 commits October 30, 2020 15:10
Add options for foreign key attributes
Update test comment to reflect open issue
…s table

to fix issue #87, #275 (add user/server connection to jobs table).

ideally, connection ID would be directly available via MyM; for the moment,
hack in a query to retrieve connection ID on connection and use this for
jobs table logging.
@guzman-raphael
Copy link
Collaborator

@ixcat You will need to target a stage-external-storage* like branch first in order for the tests to work properly. Can you create another branch, e.g. stage-external-storage2 since the current stage is already being used fro #303?

@ixcat
Copy link
Contributor Author

ixcat commented Oct 30, 2020

ah right, got confused. will readjust.

@ixcat ixcat changed the base branch from external-storage to stage-external-storage-jobid October 30, 2020 21:33
ixcat and others added 5 commits October 30, 2020 17:03
Add options for foreign key attributes
TestPopulate currently tests 2x populate calls -

  - 1x in regular populate mode - in this case, SessionAnalysis
    logs the session_id as the 'session_analysis' field
  - 1x in paralell populate mode - in this case, SessionAnalysis
    logs the job key from the jobs table as the 'session_analysis' field
@ixcat ixcat marked this pull request as ready for review November 7, 2020 19:25
Copy link
Collaborator

@guzman-raphael guzman-raphael left a comment

Choose a reason for hiding this comment

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

Looking good! Thanks for getting this test case together; I like it 😄.
Only a few minor nits on comment docs. Not necessary to implement exactly my suggestion but just looking to maintain some consistency.

+dj/+internal/AutoPopulate.m Outdated Show resolved Hide resolved
+dj/+internal/AutoPopulate.m Outdated Show resolved Hide resolved
+dj/Connection.m Outdated Show resolved Hide resolved
+dj/conn.m Outdated Show resolved Hide resolved
@guzman-raphael guzman-raphael merged commit c92cff0 into datajoint:stage-external-storage-jobid Nov 27, 2020
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.

4 participants