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

Feat/Add notification object attributes #195

Merged
merged 2 commits into from
Feb 21, 2022
Merged

Feat/Add notification object attributes #195

merged 2 commits into from
Feb 21, 2022

Conversation

carpawell
Copy link
Member

@carpawell carpawell commented Feb 14, 2022

Related to #193.

I think it should not be included in the next (2.11.0+) release, so it may wait.

@carpawell carpawell self-assigned this Feb 14, 2022
@carpawell carpawell marked this pull request as ready for review February 14, 2022 16:41
// * __NEOFS__TICK_TOPIC \
// UTF-8 string topic ID that is used for object notification
// * __NEOFS__TICK_EPOCH \
// Base10 encoded number that defines what epoch must produce
Copy link

Choose a reason for hiding this comment

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

Base10 -> decimal

@@ -117,6 +117,12 @@ message Header {
// Marks smaller parts of a split bigger object
// * __NEOFS__EXPIRATION_EPOCH \
// Tells GC to delete object after that epoch
// * __NEOFS__TICK_TOPIC \
Copy link

Choose a reason for hiding this comment

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

It' better to put it after TICK_EPOCH attribute, to simplify understanding.

// * __NEOFS__TICK_EPOCH \
// Base10 encoded number that defines what epoch must produce
// object notification (`0` value produces notification right
// after object put)
Copy link

Choose a reason for hiding this comment

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

I'd also add a note that the notification body will contain object address in UFT-8 string.

Copy link
Member Author

Choose a reason for hiding this comment

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

thought such details should be in spec, but added

Copy link

Choose a reason for hiding this comment

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

NeoFS Spec is automatically generated from this API definitions. =)

Copy link
Member Author

@carpawell carpawell Feb 15, 2022

Choose a reason for hiding this comment

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

i meant a few manually written words about notifications (that is why this PR does not close the issue)

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@realloc realloc merged commit 1842305 into nspcc-dev:master Feb 21, 2022
@carpawell carpawell deleted the feat/add-notification-obj-attributes branch February 21, 2022 12:30
cthulhu-rider added a commit that referenced this pull request Feb 22, 2024
This reverts commit f67442d.

Notification attributes turned out to be unusable. Nobody uses them, so
they are purged.

Closes #279.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
cthulhu-rider added a commit that referenced this pull request Feb 26, 2024
This reverts commit f67442d.

Notification attributes turned out to be unusable. Nobody uses them, so
they are purged.

Closes #279.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
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.

4 participants