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

Add URL support to D.O. load task (#1807) #1808

Merged
merged 1 commit into from
May 2, 2024
Merged

Conversation

mcantelon
Copy link
Member

@mcantelon mcantelon commented May 2, 2024

Added functionality, to the digital object load task, to import URLs in addition to files.

Did minor cleanup and improved CLI help to include a list of valid CSV columns.

@mcantelon mcantelon force-pushed the dev/do-load-url-support branch from 0064781 to fbdc92c Compare May 2, 2024 02:51
@mcantelon mcantelon added the Type: feature New functionality. label May 2, 2024
@mcantelon mcantelon force-pushed the dev/do-load-url-support branch from fbdc92c to b6a7596 Compare May 2, 2024 02:58
Copy link
Member

@djjuhasz djjuhasz left a comment

Choose a reason for hiding this comment

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

Looking good @mcantelon. I have a few minor suggestions but no blockers. 👌

EOF;
$this->detailedDescription = "Load a CSV list of digital objects\n\n";

$this->detailedDescription .= "Valid CSV columns are '".self::PATH_COLUMN."' and one of: '".implode("', '", self::IO_SPECIFIER_COLUMNS)."'";
Copy link
Member

@djjuhasz djjuhasz May 2, 2024

Choose a reason for hiding this comment

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

I think using sprintf() here is easier to read than string concatenation:

$this->detailedDescription .= sprintf(
  "Valid CSV columns are '%s' and one of: '%s'", 
  self::PATH_COLUMN, 
  implode("', '", self::IO_SPECIFIER_COLUMNS),
);

(line breaks optional)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call!

$valid = in_array(self::PATH_COLUMN, $columns);

// Second check for existance of an information object specifier column
if ($valid) {
Copy link
Member

@djjuhasz djjuhasz May 2, 2024

Choose a reason for hiding this comment

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

This could be shortened using array_intersect()

if ($valid) {
  $valid = count(array_intersect(self::IO_SPECIFIER_COLUMNS, $columns)) > 0;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Slick!


$filename = basename($path);
// If it's not a file, assume it's a URL and dismiss if invalid
if (!filter_var($url_or_path, FILTER_VALIDATE_URL)) {
Copy link
Member

Choose a reason for hiding this comment

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

PHP you so fancy! 🤩

}

// Check if URL exists
$headers = @get_headers($url_or_path);
Copy link
Member

Choose a reason for hiding this comment

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

Fancy x 10! 🤯

@mcantelon
Copy link
Member Author

Thanks @djjuhasz ! Good suggestions!

@mcantelon mcantelon force-pushed the dev/do-load-url-support branch 2 times, most recently from 622f3e9 to 76f3fec Compare May 2, 2024 23:21
Added functionality, to the digital object load task, to import URLs in
addition to files.

Did minor cleanup and improved CLI help to include a list of valid CSV
columns.
@mcantelon mcantelon force-pushed the dev/do-load-url-support branch from 76f3fec to c2bad00 Compare May 2, 2024 23:23
@mcantelon mcantelon merged commit 778beab into qa/2.x May 2, 2024
6 checks passed
@mcantelon mcantelon deleted the dev/do-load-url-support branch May 2, 2024 23:24
@anvit anvit added this to the 2.8.2 milestone May 16, 2024
@anvit anvit linked an issue May 16, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: feature New functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Add URL asset support to digital object load task
3 participants