-
Notifications
You must be signed in to change notification settings - Fork 5
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
Opening outside the inception year #421
Conversation
@@ -496,9 +496,15 @@ | |||
"\n ", | |||
"\n var dataNodesImported = table.Rows.Select(x => x.Field<string>(nameof(RawVariable.DataNode))).ToHashSet();", | |||
"\n var dataNodesDefined = await targetDataSource.Query<GroupOfContract>().Where(x => dataNodesImported.Contains(x.SystemName)).ToArrayAsync();", | |||
"\n var dataNodeStatesDefined = await targetDataSource.Query<DataNodeState>().Select(x => x.DataNode).ToArrayAsync();", | |||
"\n var dataNodeStatesDefined = await targetDataSource.Query<DataNodeState>().Select(x => new {DataNode = x.DataNode, InceptionYear = x.Year}).ToArrayAsync();", |
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.
Can you add (in the PR or in the issue) a description of what is wrong in the code, and how this implementation fixes it?
Currently, I fail to see it... thanks
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.
Well, let me maybe explain right here. If you carefully examine the code, you will discover that until now we never did this check in the first place, so not surprisingly this import succeeded.
So to fix I chose this place, where we query the data note state anyway. Data node state carries the information about the the inception year, which is a year, so I just expand here the query, to keep the information about the data node and the inception year. Note that the year of the args, that arrive here, carry the information about the opening year, as it comes to us from the opening file. So at the next step, if the import format is opening, I generate an enumerable with all the data nodes that have the opening outside the inception year, and if this enumerable is not empty, log an error for each entry. The data DataNodeFactoryAsync is called by the opening importer at the very early stage (line 9), at line 10 it discovers that the that the activity has errors and exists. Does it make sense?
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.
Move this validation into the parser and exploit the storage data.
Closing this PR. |
Logs an error is an opening is outside of the inception year