Skip to content

GenericRecord Feature has implemented. [API-1164][API-1853]#1086

Merged
OzanCansel merged 10 commits intohazelcast:masterfrom
OzanCansel:API-1164
Mar 10, 2023
Merged

GenericRecord Feature has implemented. [API-1164][API-1853]#1086
OzanCansel merged 10 commits intohazelcast:masterfrom
OzanCansel:API-1164

Conversation

@OzanCansel
Copy link
Copy Markdown
Contributor

@OzanCansel OzanCansel commented Feb 1, 2023

This pr is based on API-1327 branch. So a user which only wants to see changes related this PR must do a diff to API-1327 branch.

  • generic_record_builder class is added.
  • generic_record class is added.
  • Supports both write/read for a generic_record, schema distribution, serialization etc. are implemented.
  • It only supports compact serialization for generic_record.
  • const overloads of methods are added in generic_record. For low memory footprint types, it returns by value. For others it returns const T&.
  • For non-const method overloads of generic_record, it allows to modify the value via reference.
  • Methods below are implemented in generic_record
  • generic_record is copyable and movable.
  • generic_record_builder is both movable and copyable, in case it is moved, the moved from object is left at invalid state and not usable.

For detailed explanation look attached TDD to API-1853

@OzanCansel OzanCansel changed the title Api 1164 GenericRecord PoC has completed. [API-1164] Feb 1, 2023
@OzanCansel OzanCansel self-assigned this Feb 1, 2023
@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test FAILed.

@OzanCansel OzanCansel changed the title GenericRecord PoC has completed. [API-1164] GenericRecord PoC has completed. [API-1747] Feb 3, 2023
@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test FAILed.

@OzanCansel OzanCansel changed the title GenericRecord PoC has completed. [API-1747] GenericRecord PoC has completed. [API-1164] Feb 10, 2023
@OzanCansel OzanCansel changed the title GenericRecord PoC has completed. [API-1164] GenericRecord Feature has completed. [API-1164] Feb 10, 2023
@OzanCansel OzanCansel added this to the 5.2.0 milestone Feb 10, 2023
@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test FAILed.

@OzanCansel OzanCansel changed the title GenericRecord Feature has completed. [API-1164] GenericRecord Feature has implemented. [API-1164] Feb 10, 2023
@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test FAILed.

Copy link
Copy Markdown
Collaborator

@ihsandemir ihsandemir left a comment

Choose a reason for hiding this comment

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

to be continued

akeles85
akeles85 previously approved these changes Mar 8, 2023
@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test PASSed.

1 similar comment
@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test PASSed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test PASSed.

Comment on lines +933 to +938
if (already_built_) {
BOOST_THROW_EXCEPTION(exception::hazelcast_serialization{
boost::str(boost::format("Illegal to write after record is "
"built. {field : %1%, kind : %2%}") %
field_name % kind) });
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this missing for Java client? If so, can you send a PR to Java as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I remembered, it doesn't exist in Java. It needs to be implemented first at Java side. Do you want me to open an issue for it ?

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test PASSed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test FAILed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test PASSed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test PASSed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test PASSed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test PASSed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test PASSed.

ihsandemir
ihsandemir previously approved these changes Mar 10, 2023
field_kind get_field_kind(const std::string& field_name) const;

/**
* @param fieldName the name of the field
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @param fieldName the name of the field
* @param field_name the name of the field

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

}

generic_record_builder::generic_record_builder(
std::shared_ptr<pimpl::schema> boundary,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
std::shared_ptr<pimpl::schema> boundary,
std::shared_ptr<pimpl::schema> record_schema,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Windows test PASSed.

@devOpsHazelcast
Copy link
Copy Markdown
Contributor

Linux test PASSed.

@OzanCansel OzanCansel merged commit 628abe2 into hazelcast:master Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants