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

[V3 Config] Require custom group initialization before usage #2545

Merged
merged 8 commits into from Apr 5, 2019

Conversation

@tekulvw
Copy link
Member

commented Apr 3, 2019

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

This is a breaking change when using Config's custom groups feature. In order to safely and effectively convert data between data backends we must know how to interpret custom groups. Prior to this PR that was impossible because we wouldn't know the length of the custom group's primary key. The primary key is the list of identifiers passed into Config.custom().

This PR adds the Config.init_custom() method which allows you to register the length of each custom group's primary key. This method must be called before register_custom() or custom(). The information stored in config is written to disk the first time a get/set/clear happens.

tekulvw added some commits Apr 3, 2019

@tekulvw tekulvw requested a review from Twentysix26 as a code owner Apr 3, 2019

@tekulvw

This comment has been minimized.

Copy link
Member Author

commented Apr 4, 2019

Information will not be stored to disk in this PR. I need to make another PR to change bot.add_cog into a coroutine first.

@mikeshardmind
Copy link
Member

left a comment

Config objects need to be singletons with this change. The design direction this is going will require it here for some sanity instead of just at the driver.

Modification to object creation preferred over just the factory methods from me on this.

This also breaks the entire dataconverter cog. I'm fine with the removal of that cog until it can be rewritten in a more appropriate manner for future use.

tekulvw added some commits Apr 5, 2019

@mikeshardmind
Copy link
Member

left a comment

Works as intended. Is breaking. Needs an announcement in #v3-burndown .

@Kowlin

Kowlin approved these changes Apr 5, 2019

@tekulvw tekulvw merged commit 0852d1b into Cog-Creators:V3/develop Apr 5, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details
@mikeshardmind mikeshardmind referenced this pull request Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.