Skip to content

Add Support for Partial Exports and Imports #505

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

phalestrivir
Copy link
Contributor

@phalestrivir phalestrivir commented Apr 10, 2025

This PR enables the library to return partial exports when errors occur during exports and imports. This allows for the CLI to still use the partial exports for exports that were successful. In other words, this PR addresses this issue.

Note that I only updated a few of the export and import functions. There are other export and import functions where we could potentially do this as well, although it's something that would take a lot of time to do (and in some cases it doesn't make sense to do it since all the config gets exported in one REST call, so we wouldn't be able to return partial results in those cases), so I focused on the main ones where it would be important to support this. These include scripts, journeys, idm config, and the full config functions.

Generally, the way it works is to keep an array of errors that occur during exports/imports, and at the end throw a FrodoError with those errors in addition to the partial export object(s). The CLI can then decide when an error occurs if they want to utilize the partial exports when the error is thrown.

To follow the rule for the library, which is that it should always throw when encountering errors, anywhere in the functions I modified where it was printing errors were changed to throw errors instead. There are many other places which are still printing errors throughout the library however, so this is another refactoring change that will need to be made across the whole library in the future.

See my PR in the CLI to see the changes I made there to make it work with the changes in the lib.

@phalestrivir
Copy link
Contributor Author

After talking with @hfranklin, I'm going to refactor the PR to use callbacks instead, similar to how something like fs.readFile works. For example, with fs.readFile we can pass in a callback to the function like (err, data) => { ... }, where err is populated if there is an error during the read, otherwise data is populated. We will use this for handling the partial exports instead of doing it as part of the FrodoError. We will pass in similar callbacks to the export/import functions so we can handle it similarly. If no callback is provided, the functions will work as they usually do by throwing an error if one occurs, otherwise returning all the results.

@vscheuber
Copy link
Contributor

@phalestrivir please let me know when this PR is ready to merge.

@phalestrivir phalestrivir force-pushed the feature/support-partial-exports branch from 65e22a5 to ebf6df8 Compare May 12, 2025 17:56
@phalestrivir phalestrivir reopened this May 12, 2025
@phalestrivir
Copy link
Contributor Author

@vscheuber The PR should be ready to merge now. I updated it to use callbacks instead of the way we were doing it before using FrodoError. The CLI PR was updated as well

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