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

refactor!(share): integrate new Namespace type #2376

Closed
wants to merge 22 commits into from

Conversation

Wondertan
Copy link
Member

@Wondertan Wondertan commented Jun 16, 2023

TODO:

  • Document changes on the PR for reviewers and for commit message
  • Use of ValidateDataNamespace
  • Document in issues further actionables

Based on #2367
Closes #2301
Blocked on #2256 (because new rules on namespaces in existing tests cause to trigger not found within namespace range path logic, which we don't handle yet)

@Wondertan Wondertan added area:shares Shares and samples kind:break! Attached to breaking PRs kind:refactor Attached to refactoring PRs labels Jun 16, 2023
@Wondertan Wondertan self-assigned this Jun 16, 2023
@Wondertan Wondertan changed the title share: introduce Namespace type refactor!(share): use new Namespace type Jun 16, 2023
@@ -20,7 +19,7 @@ const (
namespacedDataEndpoint = "/namespaced_data"
)

var nIDKey = "nid"
Copy link
Member Author

Choose a reason for hiding this comment

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

Forgot to change the actual string and it's breaking....

Don't have processing power rn to assess whether this worth a breakage here or not

Copy link
Member Author

@Wondertan Wondertan Jun 19, 2023

Choose a reason for hiding this comment

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

We are not deprecating this, so it might be worth it. Does it need changes besides go-cnc?

@Wondertan Wondertan force-pushed the hlib/share/namespace-integration branch from 1854a3a to 7bcece5 Compare June 16, 2023 17:07
@Wondertan Wondertan changed the title refactor!(share): use new Namespace type refactor!(share): integrate new Namespace type Jun 16, 2023
@Wondertan
Copy link
Member Author

Wondertan commented Jun 19, 2023

Currently contains all the changes from #2256 to check that all the tests are passing with it.

@Wondertan
Copy link
Member Author

Closed in favor of #2388. GH does not see new commits.

@Wondertan Wondertan closed this Jun 22, 2023
github-merge-queue bot pushed a commit that referenced this pull request Jun 27, 2023
## Notable Changes
- `namespace.ID` to  `share.Namespace`
- Changes every comment namespace ID mentioning to just namespace. I
renamed every such mention besides in ADRs. I don't want to touch ADRs
here, as they need a more holistic re-review and up-to-date catchup.
	- Uses all the utility methods on the type, where suitable
- Namespace constructor now only creates Blob Namespaces. For other
reserved namespaces, the predefined globals should be used.
- Uses namespace.ValidateDataNamespace everywhere data is requested.
This is guarantees we verify the namespace are 100% valid and forbids
requesting parity and padding namespaces.
- Restricts PFBs for reserved namespaces 
- Reversed the dependency from `share -> share/ipld` to `share/ipld ->
share`
- NewBlobV0 constructor. Similar to NewNamespaceV0
- `sharetest` pkg for share related testing utilities
- `edstest` pkg for eds related testing utilities

## Follow-ups
- `blobtest` pkg to generate node's blob type


## Refs
* Substitutes zombie PR
#2376. I push to the
branch, but GH does not see commits.
* Based on #2367
* Closes #2301
* Closes #2309 
* Blocked on #2256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:shares Shares and samples kind:break! Attached to breaking PRs kind:refactor Attached to refactoring PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename nID to namespace where applicable
2 participants