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

U/jrbogart/allowing duplicate relpath #161

Merged
merged 3 commits into from
Oct 27, 2024

Conversation

JoanneBogart
Copy link
Collaborator

@JoanneBogart JoanneBogart commented Oct 24, 2024

Don't require relative_path to be unique in case the dataset to be registered has old_location == None because there is no danger of overwriting.
Addresses the second part of issue #156

Copy link
Collaborator

@stuartmcalpine stuartmcalpine left a comment

Choose a reason for hiding this comment

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

Looks ok ... just a check...

  • Could raise a warming rather than printing (see line 825 for example
  • Should the archived case still behave as a warning. I thought for archived datasets, no matter what the relative path is reserved for the dataset possibly coming back

@JoanneBogart
Copy link
Collaborator Author

I've addressed your first point (thanks for the reminder about using warnings).

For the archived case, there is still no danger of anything being overwritten because the call to register isn't doing any writing. There is another problem, though, which is that, if the other dataset was archived, as a practical matter the new one is also. I could update the status to say "archived". But I think there will be other issues along these lines with datasets coming from GCRCatalogs, e.g., if one dataset is based on another and the underlying one is archived, the other one is, too, effectively. I'm going to leave this alone for now; when we implement archiving we'll need to revisit these situations.

@JoanneBogart JoanneBogart merged commit d403cfe into main Oct 27, 2024
26 checks passed
@JoanneBogart JoanneBogart deleted the u/jrbogart/allowing_duplicate_relpath branch October 27, 2024 02:23
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