You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Sidharth Mishra <si...@gmail.com> on 2021/09/15 21:37:04 UTC
Review Request 73585: ATLAS-4425: Migration import should be able to
import multiple zip files present in a particular path
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73585/
-----------------------------------------------------------
Review request for atlas, Ashutosh Mestry, Disha Talreja, Radhika Kundam, and Sarath Subramanian.
Bugs: ATLAS-4425
https://issues.apache.org/jira/browse/ATLAS-4425
Repository: atlas
Description
-------
ATLAS-442: Added support for Migration import to run on a directory with multip[le zip files
Diffs
-----
repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java 04342fa52
repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java 0a2257eae
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java d56261f78
Diff: https://reviews.apache.org/r/73585/diff/1/
Testing
-------
Manually tested with multiple zip files at a location which was imported successfully.
Thanks,
Sidharth Mishra
Re: Review Request 73585: ATLAS-4425: Migration import should be able
to import multiple zip files present in a particular path
Posted by Sidharth Mishra <si...@gmail.com>.
> On Sept. 29, 2021, 6:20 p.m., Ashutosh Mestry wrote:
> > repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java
> > Lines 260 (patched)
> > <https://reviews.apache.org/r/73585/diff/2/?file=2253180#file2253180line260>
> >
> > I think this change is not necessary given that the input will always have a typeDefinition.
This is needed as if someone forget to pass the typedef then we should not throw NPE.
- Sidharth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73585/#review223548
-----------------------------------------------------------
On Oct. 5, 2021, 11:30 p.m., Sidharth Mishra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73585/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2021, 11:30 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Disha Talreja, Radhika Kundam, and Sarath Subramanian.
>
>
> Bugs: ATLAS-4425
> https://issues.apache.org/jira/browse/ATLAS-4425
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-442: Added support for Migration import to run on a directory with multip[le zip files
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java 04342fa52
> repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java d56261f78
>
>
> Diff: https://reviews.apache.org/r/73585/diff/3/
>
>
> Testing
> -------
>
> Manually tested with multiple zip files at a location which was imported successfully.
>
>
> Thanks,
>
> Sidharth Mishra
>
>
Re: Review Request 73585: ATLAS-4425: Migration import should be able
to import multiple zip files present in a particular path
Posted by Ashutosh Mestry via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73585/#review223548
-----------------------------------------------------------
I have tested this and it works quite well.
repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java
Lines 260 (patched)
<https://reviews.apache.org/r/73585/#comment312619>
I think this change is not necessary given that the input will always have a typeDefinition.
repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java
Line 68 (original), 68 (patched)
<https://reviews.apache.org/r/73585/#comment312613>
Doing this will impact branch-0.8 to 1.0 migration. The old and new migration relies on directory vs file existence check.
repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java
Lines 93 (patched)
<https://reviews.apache.org/r/73585/#comment312614>
This won't be necessary as well.
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Line 61 (original), 66 (patched)
<https://reviews.apache.org/r/73585/#comment312615>
If the file contains wildcards (? or '*'), expand them using listFiles.
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Line 62 (original), 67 (patched)
<https://reviews.apache.org/r/73585/#comment312618>
Add check to detect archive folder can be created. If not LOG.WARN and proceed without archieving.
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Lines 94 (patched)
<https://reviews.apache.org/r/73585/#comment312616>
Modify this to take the user-specified input, expand if necessary. Sort it and then return the list of files to be imported.
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Lines 135 (patched)
<https://reviews.apache.org/r/73585/#comment312617>
Be defensive here.
if (this.archiveDir == null) {
return;
}
Doing this will add clarity.
- Ashutosh Mestry
On Sept. 23, 2021, 3:52 a.m., Sidharth Mishra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73585/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2021, 3:52 a.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Disha Talreja, Radhika Kundam, and Sarath Subramanian.
>
>
> Bugs: ATLAS-4425
> https://issues.apache.org/jira/browse/ATLAS-4425
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-442: Added support for Migration import to run on a directory with multip[le zip files
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java 04342fa52
> repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java 0a2257eae
> repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java d56261f78
>
>
> Diff: https://reviews.apache.org/r/73585/diff/2/
>
>
> Testing
> -------
>
> Manually tested with multiple zip files at a location which was imported successfully.
>
>
> Thanks,
>
> Sidharth Mishra
>
>
Re: Review Request 73585: ATLAS-4425: Migration import should be able
to import multiple zip files present in a particular path
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73585/#review223590
-----------------------------------------------------------
Fix it, then Ship it!
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Lines 71 (patched)
<https://reviews.apache.org/r/73585/#comment312694>
add a comment about fileName can support wildcards.
fileNameMightBeWithWildcard => fileName
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Lines 74 (patched)
<https://reviews.apache.org/r/73585/#comment312698>
consider moving this method to create archive directory inside getAllFilesToImport(); The parent File objects are already constructed in getAllFilesToImport().
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Lines 112 (patched)
<https://reviews.apache.org/r/73585/#comment312695>
Move line 109 and 112 inside 'if' block (after line 115). Construct this object only after checking if it is a valid directory.
also move line 117 inside 'if' block (after line 119)
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Lines 122 (patched)
<https://reviews.apache.org/r/73585/#comment312696>
eachFile => importFile
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Lines 123 (patched)
<https://reviews.apache.org/r/73585/#comment312697>
extract to method to check - isValidImportFile(importFile)
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java
Lines 162 (patched)
<https://reviews.apache.org/r/73585/#comment312699>
check if you have write/move permission on the destination archive dir as well (newFile)
- Sarath Subramanian
On Oct. 5, 2021, 4:30 p.m., Sidharth Mishra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73585/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2021, 4:30 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Disha Talreja, Radhika Kundam, and Sarath Subramanian.
>
>
> Bugs: ATLAS-4425
> https://issues.apache.org/jira/browse/ATLAS-4425
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-442: Added support for Migration import to run on a directory with multip[le zip files
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java 04342fa52
> repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java d56261f78
>
>
> Diff: https://reviews.apache.org/r/73585/diff/3/
>
>
> Testing
> -------
>
> Manually tested with multiple zip files at a location which was imported successfully.
>
>
> Thanks,
>
> Sidharth Mishra
>
>
Re: Review Request 73585: ATLAS-4425: Migration import should be able
to import multiple zip files present in a particular path
Posted by Sidharth Mishra <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73585/
-----------------------------------------------------------
(Updated Oct. 7, 2021, 11:59 p.m.)
Review request for atlas, Ashutosh Mestry, Disha Talreja, Radhika Kundam, and Sarath Subramanian.
Bugs: ATLAS-4425
https://issues.apache.org/jira/browse/ATLAS-4425
Repository: atlas
Description
-------
ATLAS-442: Added support for Migration import to run on a directory with multip[le zip files
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java 04342fa52
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java d56261f78
Diff: https://reviews.apache.org/r/73585/diff/4/
Changes: https://reviews.apache.org/r/73585/diff/3-4/
Testing
-------
Manually tested with multiple zip files at a location which was imported successfully.
Thanks,
Sidharth Mishra
Re: Review Request 73585: ATLAS-4425: Migration import should be able
to import multiple zip files present in a particular path
Posted by Sidharth Mishra <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73585/
-----------------------------------------------------------
(Updated Oct. 5, 2021, 11:30 p.m.)
Review request for atlas, Ashutosh Mestry, Disha Talreja, Radhika Kundam, and Sarath Subramanian.
Bugs: ATLAS-4425
https://issues.apache.org/jira/browse/ATLAS-4425
Repository: atlas
Description
-------
ATLAS-442: Added support for Migration import to run on a directory with multip[le zip files
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java 04342fa52
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java d56261f78
Diff: https://reviews.apache.org/r/73585/diff/3/
Changes: https://reviews.apache.org/r/73585/diff/2-3/
Testing
-------
Manually tested with multiple zip files at a location which was imported successfully.
Thanks,
Sidharth Mishra
Re: Review Request 73585: ATLAS-4425: Migration import should be able
to import multiple zip files present in a particular path
Posted by Ashutosh Mestry via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73585/#review223550
-----------------------------------------------------------
User specifies:
atlas.migration.data.filename=/tmp/atlas-data/golden-?-of-2-atlas.zip
The directory contains:
golden-1-of-2-atlas.zip
golden-2-of-2-atlas.zip
Algo:
- RW permissions for /tmp/atlas-data/archive.
- If permissions exist, sucessfully imported files are moved to archive directory.
- If not, LOG.WARN is displayed. Implication of this is the subsequent startup in migration mode will restart import from the beginning.
- - Wildcard gets expanded.
- Files are enumerated.
- They are sequentially fed to the importer.
- Successful files are moved to /tmp/atlas-data/archive folder.
- Ashutosh Mestry
On Sept. 23, 2021, 3:52 a.m., Sidharth Mishra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73585/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2021, 3:52 a.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Disha Talreja, Radhika Kundam, and Sarath Subramanian.
>
>
> Bugs: ATLAS-4425
> https://issues.apache.org/jira/browse/ATLAS-4425
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-442: Added support for Migration import to run on a directory with multip[le zip files
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java 04342fa52
> repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java 0a2257eae
> repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java d56261f78
>
>
> Diff: https://reviews.apache.org/r/73585/diff/2/
>
>
> Testing
> -------
>
> Manually tested with multiple zip files at a location which was imported successfully.
>
>
> Thanks,
>
> Sidharth Mishra
>
>
Re: Review Request 73585: ATLAS-4425: Migration import should be able
to import multiple zip files present in a particular path
Posted by Sidharth Mishra <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73585/
-----------------------------------------------------------
(Updated Sept. 23, 2021, 3:52 a.m.)
Review request for atlas, Ashutosh Mestry, Disha Talreja, Radhika Kundam, and Sarath Subramanian.
Bugs: ATLAS-4425
https://issues.apache.org/jira/browse/ATLAS-4425
Repository: atlas
Description
-------
ATLAS-442: Added support for Migration import to run on a directory with multip[le zip files
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java 04342fa52
repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java 0a2257eae
repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java d56261f78
Diff: https://reviews.apache.org/r/73585/diff/2/
Changes: https://reviews.apache.org/r/73585/diff/1-2/
Testing
-------
Manually tested with multiple zip files at a location which was imported successfully.
Thanks,
Sidharth Mishra