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

Make neptune-export cross-platform by removing hardcoded line and path separators #202

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

drossi750
Copy link

Make neptune-export cross-platform by removing hardcoded line and path separators

Signed-off-by: Rossi, Davide [email protected]

Issue #, if available:

Description of changes:

Even though this would run in a linux-like environment like a lambda, developing and running on a Windows machine yields failing tests. There is no change in the behavior of the application, just making sure that the same execution on different OS leads to the same result.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@beebs-systap
Copy link
Member

@triggan Can you take a quick look at this one?

@@ -94,7 +98,7 @@ public void run() {
directories.queriesResource();

CsvPrinterOptions csvPrinterOptions = CsvPrinterOptions.builder().setIncludeTypeDefinitions(includeTypeDefinitions).build();
JsonPrinterOptions jsonPrinterOptions = JsonPrinterOptions.builder().setStrictCardinality(true).build();
JsonPrinterOptions jsonPrinterOptions = JsonPrinterOptions.builder().setStrictCardinality(strictCardinality).build();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a change to the default behavior of setStrictCardinality(true). Instead of changing the default behavior, can we leave strictCardinality as true and change the parameter to accept --strict-cardinality false instead?

@triggan
Copy link
Contributor

triggan commented Jul 20, 2022

I took a look at the committed changes. Everything looks good except for the change in default behavior for JSON formatting for properties with single values. I think adding the ability to remove the square brackets for properties with single properties makes sense and is needed, but we likely need to make that a configurable change versus changing that to be the default. This thinking is based on the fact that neptune-export is a dependency for many other workloads at this point.

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.

3 participants