Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Split AS stores #2901

Merged
merged 3 commits into from
Feb 27, 2018
Merged

Split AS stores #2901

merged 3 commits into from
Feb 27, 2018

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Feb 21, 2018

This still has a dependency on _get_events, so we should probably wait until I've factored that out before looking at this.

Includes #2902



class ApplicationServiceTransactionStore(ApplicationServiceTransactionWorkerStore):
pass
Copy link
Member Author

Choose a reason for hiding this comment

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

Should I remove this and replace it everywhere with ApplicationServiceTransactionWorkerStore?

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't, because: (a) consistency with the other stores (b) at some point we might want to do something that needs to not be on the workers (c) it would be a faff

Copy link
Member

Choose a reason for hiding this comment

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

a comment to clarify mightn't hurt.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm



class ApplicationServiceTransactionStore(ApplicationServiceTransactionWorkerStore):
pass
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't, because: (a) consistency with the other stores (b) at some point we might want to do something that needs to not be on the workers (c) it would be a faff



class ApplicationServiceTransactionStore(ApplicationServiceTransactionWorkerStore):
pass
Copy link
Member

Choose a reason for hiding this comment

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

a comment to clarify mightn't hurt.

@richvdh richvdh assigned erikjohnston and unassigned richvdh Feb 26, 2018
@erikjohnston erikjohnston merged commit c576078 into develop Feb 27, 2018
@erikjohnston erikjohnston deleted the erikj/split_as_stores branch March 5, 2018 15:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants