-
Notifications
You must be signed in to change notification settings - Fork 4
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
Implement analysis migration for VDI #83
Conversation
public static final String UD_DATASET_ID_PREFIX = "EDAUD_"; | ||
|
||
private Map<String, String> legacyIdToVdiId; | ||
private int missingFromVdiCount = 0; |
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.
Probably want AtomicInteger here. I don't think I've consistently used it in the past and it's sorta been ok, but...
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.
Forgot the executor was multi-threaded, fixed
final String vdiDatasetId = UD_DATASET_ID_PREFIX + vdiId; | ||
|
||
// Create a copy with just the dataset ID updated to VDI counterpart. | ||
AnalysisRow out = new AnalysisRow(nextRow.getAnalysisId(), vdiDatasetId, nextRow.getDescriptor(), |
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.
You don't have to do this here. You could just make a datasetId setter and modify nextRow. The incoming object is a throwaway to the "framework". Might save time.
AnalysisRow out = new AnalysisRow(nextRow.getAnalysisId(), vdiDatasetId, nextRow.getDescriptor(), | ||
nextRow.getNumFilters(), nextRow.getNumComputations(), nextRow.getNumVisualizations()); | ||
|
||
return new TableRowInterfaces.RowResult<>(out); |
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.
Want to set the result object's update flag to true here unless _writeToDb is false.
|
||
// Append UD prefix to VDI ID. The prefix is prepended in the view that maps stable VDI IDs to the unstable study | ||
// ID, which is the currency of EDA. | ||
final String vdiDatasetId = UD_DATASET_ID_PREFIX + vdiId; |
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 thought we were planning to change the previx to VDIID_ or something once we moved to it? Weren't there multiple hour-long conversations about VD vs VDID vs VDIID?
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 is just for referring to EDA user datasets that are managed by VDI. I think keeping it as UD should be fine. We discussed in scrum this morning.
3, | ||
4, | ||
5)); | ||
Assert.assertEquals("EDAUD_123XyZ", result.getRow().getDatasetId()); |
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.
So maybe also check that update flag is true here. And can add a case where it should not be true.
Overview
Analysis migration plugin for UDIS -> VDI
Testing
Pending VDI deployment to dev