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

Is this behavior in alias-navigation intentional? #452

Open
mslw opened this issue May 4, 2024 · 2 comments
Open

Is this behavior in alias-navigation intentional? #452

mslw opened this issue May 4, 2024 · 2 comments

Comments

@mslw
Copy link
Collaborator

mslw commented May 4, 2024

This is something I noticed related to the alias navigation introduced in #430.

When having the Inspect console open while browsing the Catalog, I noticed 404'd GET requests being made to JSON files with valid dataset IDs, but nonsensical versions (not seen anywhere in the dataset).

They are coming from this piece of code, introduced in 3f3963d:

dataset_id_path = getFilePath(this.selectedDataset.dataset_id)
fetch(dataset_id_path)
.then((response) => {
if(response.status == 404) {
this.selectedDataset.has_id_path = false
} else {
this.selectedDataset.has_id_path = true
}
})
.catch(error => {
// do nothing
})

The "version" (json file name) is coming from getFilePath(this.selectedDataset.dataset_id), defined here in 7dfbda7:

function getFilePath(dataset_id, dataset_version, path, ext = ".json") {
// Get node file location from dataset id, dataset version, and node path
// using a file system structure similar to RIA stores
// - dataset_id is required, all the other parameters are optional
// - dataset_id could either be an actual dataset ID or an alias
file = metadata_dir + "/" + dataset_id
blob = dataset_id
if (dataset_version) {
file = file + "/" + dataset_version;
blob = blob + "-" + dataset_version;
// path to file only makes sense with the context of a dataset id AND version
if (path) {
blob = blob + "-" + path;
}
blob = md5(blob);
blob_parts = [blob.substring(0, SPLIT_INDEX), blob.substring(SPLIT_INDEX)];
return file + "/" + blob_parts[0] + "/" + blob_parts[1] + ext;
} else {
blob = md5(blob);
return file + "/" + blob + ext;
}
}

With the getFilePath(this.selectedDataset.dataset_id), the version is null, we fall into else clause, and the "version" (filename) becomes an md5 hash of the dataset ID.

  • is returning the md5 of the dataset ID intentional?
  • can there be an alternative to making an additional GET request any time a dataset page is opened?
  • can the request errors be silenced to avoid confusion?

Sorry if this is a little rambling, but it confused me while debugging an unrelated thing, and I wanted to jot this down before I forget.

@jsheunis
Copy link
Member

Sorry for taking so long to respond, this issue slipped past me.

is returning the md5 of the dataset ID intentional?

Yes, because that gives us the filename of the "concept-ID redirect file", which will point to a specific dataset id+version metadata file. to be rendered If the alias was used for navigation, the alias would be the argument that is passed to the same function (instead of the ID), and it would give us the "alias redirect file".

can there be an alternative to making an additional GET request any time a dataset page is opened?

it could perhaps be possible, but at the time when I wrote this code, I couldn't think of a different/better way. If you have ideas, feel free to share.

can the request errors be silenced to avoid confusion?

Yes, I did this a while back for some of the requests, but apparently didn't propagate to all fetch calls. I will need to check where and how I did this. Definitely a good idea, to silence some expected "errors".

@jsheunis
Copy link
Member

jsheunis commented Jun 7, 2024

The questions were answered and the remaining issue is the console error when being unable to load a resource. These posts suggest it is unwise to do so:

The internet suggests the best way to check for the existence of a file/resource on the server is to use fetch with method "HEAD", like this: https://github.com/datalad/datalad-catalog/blob/main/datalad_catalog/catalog/assets/app_globals.js#L95-L105

I must check whether this method: 1 returns a console error, and 2 is used in all sensible places in the catalog code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants