-
Notifications
You must be signed in to change notification settings - Fork 16
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
Fix bug in dataservice use of obj store settings #610
Fix bug in dataservice use of obj store settings #610
Conversation
Fixing usage of ServiceSetting object in service's main lifespan() func, as use was of earlier names for properties, which were changed in code review but not updated here; also bumping package version.
@christophertubbs, since Austin probably won't be able today, when you have 5 minutes, could you give this a quick review? It's very small, fixing some uses of attributes of ServiceSettings (that isn't change and can be found here) in the data service, which I think we just missed when we modified some the names during that PR. |
Any way I can reasonably complain about I imagine this pickiness is out of scope though. Let me know if that's the case. |
A fair question, @christophertubbs, but there is a reason behind it (albeit convoluted). The Python attributes need to match the names of some configured Docker secrets in the main stack so that the ServiceSettings class can load those attributes' values from the secrets. The main stack Docker secrets - The object_store Docker stack secrets - |
So... you're saying That makes sense, and that makes naming complaints even more out of scope than I thought. I'll throw a tantrum about that later, lol. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
But seriously, good fix.
Fixing usage of ServiceSetting object in service's main lifespan() func, as use was of earlier names for properties, which were changed in code review but not updated here; also bumping package version.