Skip to content

fix: validator can run in any direcotry - #78

Open
nanjiangshu wants to merge 4 commits into
mainfrom
fix/validator-can-run-in-any-folder
Open

fix: validator can run in any direcotry#78
nanjiangshu wants to merge 4 commits into
mainfrom
fix/validator-can-run-in-any-folder

Conversation

@nanjiangshu

Copy link
Copy Markdown
Contributor

This PR fixes the issue that the validator script has to be run at where the script is located.

Changes made

  • Prefix hard coded directories and file targets with $WORKDIR for isolated path resolution
  • Update k8s job setup to read config/data relative to $WORKDIR
  • Invoke pre-built bpctl directly from PATH instead of building locally with go build
  • updated readme: highlight recommended standard workflow: run with --dry-run first to
    check validation, followed by a second run without --dry-run for ingestion

…ecks

- Add dependency check for `bpctl` to ensure CLI availability
- Prefix temporary directories and file targets with `$WORKDIR` for isolated path resolution
- Update k8s job setup to read config/data relative to `$WORKDIR` and `$SCRIPT_DIR`
- Invoke pre-built `bpctl` directly from PATH instead of building locally with `go build`
- Highlight recommended standard workflow: run with --dry-run first to
  check validation, followed by a second run without --dry-run for
  ingestion
- Add recommendation to create and use a dedicated dataset folder (e.g.
  `DATASET_aaaaaaa`) as WORKDIR
- Document the dependency on `bpctl` being present in PATH

@kostas-kou kostas-kou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good changes

Comment thread validator/validator.sh
<(printf '%s\n' "$extra_inbox_relative") \
<(paste \
<(echo "$all_thumbnail_files") \
<(echo "$all_thumbnail_relative" | sed 's|LANDING_PAGE/THUMBNAILS/||')))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you tried this? I think you need that sed for the 'relative' list

Comment thread validator/validator.sh
Comment on lines -699 to -706
# Function for checking the metadata and inbox files.
# - Gets a list of all the dataset filepaths in the inbox
# - Counts only the files in the IMAGES folder
# - Parses the filenames from metadata and counts them
# - Checks if the metadata list is empty and if it is, then exits
# - Checks if the number of files in the inbox and in the metadata are equal
# - If there are equal number of files, then it checks if the filenames from metadata exist in the inbox
# - If they are not equal, it prints the extra or missing files and exits

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I find the function summaries useful, was there a reason to completely remove them?

Comment thread validator/validator.sh
Comment on lines 491 to 526
@@ -510,63 +545,56 @@ function validate_structure {

# Function for downloading the xml metadata files from the inbox
function get_xml_files {
mkdir -p xml-files
mkdir -p "$WORKDIR/xml-files"
cecho yellow "Getting xml files ..."
for bucket in "${INBOX_BUCKETS[@]}"; do
metadata_path=$(s3cmd_command ls "s3://${bucket}/${user}/${dataset}/" 2>/dev/null | grep -i METADATA | awk '{print $2}')
private_path=$(s3cmd_command ls "s3://${bucket}/${user}/${dataset}/" 2>/dev/null | grep -i PRIVATE | awk '{print $2}')
if [[ -n "$metadata_path" ]]; then
s3cmd_command get "$metadata_path" --recursive xml-files/ >/dev/null 2>&1
s3cmd_command get "$metadata_path" --recursive "$WORKDIR/xml-files/" >/dev/null 2>&1
fi
if [[ -n "$private_path" ]]; then
s3cmd_command get "$private_path" --recursive xml-files/ >/dev/null 2>&1
s3cmd_command get "$private_path" --recursive "$WORKDIR/xml-files/" >/dev/null 2>&1
fi
done
if [[ "$LANDING_PAGE" == "true" ]]; then
for bucket in "${INBOX_BUCKETS[@]}"; do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

was this needed to run with the workdir addition?

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