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

Msgpack Overhaul #354

Merged
merged 23 commits into from Aug 15, 2019
Merged

Msgpack Overhaul #354

merged 23 commits into from Aug 15, 2019

Conversation

dgasmith
Copy link
Contributor

@dgasmith dgasmith commented Aug 7, 2019

Description

This PR accomplishes the following:

  • Introduces ProtoModel as the base to all models.
  • Alters the storage socket to store msg pack for speed.
  • Changes default FractalClient submission to msgpack based rather than msgpack based.

This will not pass CI until a number of other PR's and releases are in from Engine/Elemental. This does however pass locally (!), somewhat surprised by this.

Best part: Primary SQL msgpack overhaul was 27 lines of code. A clear win for OOP and adapters that we might want to use as an example. @sjayellis @doaa-altarawy

Status

  • Changelog updated
  • Ready to go

@dgasmith dgasmith added Enhancement Project enhancement discussion Database Related to the database storage layer Interface Related to the Interface layer (QCPortal) Feature Adds a new feature to the project labels Aug 7, 2019
@dgasmith dgasmith added this to the 0.9.0 milestone Aug 7, 2019
@lgtm-com
Copy link

lgtm-com bot commented Aug 7, 2019

This pull request introduces 5 alerts and fixes 1 when merging b2e4559 into c4e5245 - view on LGTM.com

new alerts:

  • 5 for Unused import

fixed alerts:

  • 1 for __eq__ not overridden when adding attributes

@dgasmith dgasmith mentioned this pull request Aug 12, 2019
3 tasks
@lgtm-com
Copy link

lgtm-com bot commented Aug 14, 2019

This pull request introduces 38 alerts and fixes 2 when merging 1f06ec8 into f3eea65 - view on LGTM.com

new alerts:

  • 35 for Unused import
  • 3 for Unused local variable

fixed alerts:

  • 2 for __eq__ not overridden when adding attributes

@dgasmith dgasmith changed the title [WIP] Msgpack Overhaul Msgpack Overhaul Aug 14, 2019
@lgtm-com
Copy link

lgtm-com bot commented Aug 14, 2019

This pull request introduces 28 alerts and fixes 2 when merging da87985 into f3eea65 - view on LGTM.com

new alerts:

  • 25 for Unused import
  • 3 for Unused local variable

fixed alerts:

  • 2 for __eq__ not overridden when adding attributes

@lgtm-com
Copy link

lgtm-com bot commented Aug 15, 2019

This pull request introduces 27 alerts and fixes 2 when merging c9f9c80 into f3eea65 - view on LGTM.com

new alerts:

  • 25 for Unused import
  • 2 for Unused local variable

fixed alerts:

  • 2 for __eq__ not overridden when adding attributes

@codecov
Copy link

codecov bot commented Aug 15, 2019

Codecov Report

Merging #354 into master will decrease coverage by 13.38%.
The diff coverage is 87.67%.

1 similar comment
@codecov
Copy link

codecov bot commented Aug 15, 2019

Codecov Report

Merging #354 into master will decrease coverage by 13.38%.
The diff coverage is 87.67%.

@dgasmith
Copy link
Contributor Author

Merging this in as it is blocking other PR's. @doaa-altarawy Please review in detail when you can.

@dgasmith dgasmith merged commit 23deed9 into MolSSI:master Aug 15, 2019
@dgasmith dgasmith deleted the msgpack branch August 15, 2019 14:25
@lgtm-com
Copy link

lgtm-com bot commented Aug 15, 2019

This pull request introduces 27 alerts and fixes 2 when merging 8718ddc into f3eea65 - view on LGTM.com

new alerts:

  • 25 for Unused import
  • 2 for Unused local variable

fixed alerts:

  • 2 for __eq__ not overridden when adding attributes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Database Related to the database storage layer Enhancement Project enhancement discussion Feature Adds a new feature to the project Interface Related to the Interface layer (QCPortal)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant