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

Feature/rdmp 73 holdouts extractions #1648

Draft
wants to merge 28 commits into
base: develop
Choose a base branch
from

Conversation

JFriel
Copy link
Collaborator

@JFriel JFriel commented Oct 13, 2023

rdmp-73-extraction
Adds the extraction half of the holdout piece.
Allows for holdouts to be generated at extraction time and be stored in an arbitrary file.
Holdouts can be a set number, a percentage, can optionally require to be between two dates and can be further filtered

@JFriel JFriel requested a review from jas88 October 13, 2023 10:01
Comment on lines +71 to +74
catch (Exception)
{
dateCell = DateTime.Parse(row.Field<string>(dateColumn), CultureInfo.InvariantCulture);
}

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
@JFriel JFriel changed the title Feature/rdmp 74 holdouts extractions Feature/rdmp 73 holdouts extractions Oct 20, 2023
@JFriel JFriel marked this pull request as ready for review October 20, 2023 12:56
if (!string.IsNullOrWhiteSpace(whereCondition))
{
DataTable dt = toProcess.Clone();
DataTable dt2 = new();

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable Warning

Disposable 'DataTable' is created but not disposed.
if (!string.IsNullOrWhiteSpace(whereCondition))
{
DataTable dt = toProcess.Clone();
DataTable dt2 = new();

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
dt2
is useless, since its value is never read.
@JFriel JFriel marked this pull request as draft October 27, 2023 15:15
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.

2 participants