Re: [openstack-dev] [cinder] [driver] DB operations
Thanks Mike for getting me these useful reviews design discussions. So as it stands now, I am trying '*provider_id*' to map OpenStack/Cinder with the driver's backend storage. I got some useful review comments from @xing-yang to try out '*provider_id'* feature enabled by below commit: https://review.openstack.org/#/c/143205/ Do let me know if '*provider_id'* approach seems reasonable ? Regards, Amit *CloudByte Inc.* http://www.cloudbyte.com/ On Thu, Dec 25, 2014 at 1:46 AM, Mike Perez thin...@gmail.com wrote: On 06:05 Sat 20 Dec , Duncan Thomas wrote: No, I mean that if drivers are going to access database, then they should do it via a defined interface that limits what they can do to a sane set of operations. I'd still prefer that they didn't need extra access beyond the model update, but I don't know if that is possible. Duncan Thomas On Dec 19, 2014 6:43 PM, Amit Das amit@cloudbyte.com wrote: Thanks Duncan. Do you mean hepler methods in the specific driver class? On 19 Dec 2014 14:51, Duncan Thomas duncan.tho...@gmail.com wrote: So our general advice has historical been 'drivers should not be accessing the db directly'. I haven't had chance to look at your driver code yet, I've been on vacation, but my suggestion is that if you absolutely must store something in the admin metadata rather than somewhere that is covered by the model update (generally provider location and provider auth) then writing some helper methods that wrap the context bump and db call would be better than accessing it directly from the driver. Duncan Thomas On Dec 18, 2014 11:41 PM, Amit Das amit@cloudbyte.com wrote: I've expressed in past reviews that we should have an interface that limits drivers access to the database [1], but received quite a bit of push back in Cinder. I recommend we stick to what has been decided, otherwise, Amit you should spend some time on reading the history of this issue [2] from previous meetings and start a rediscussion on it in the next meeting [3]. Not discouraging it, but this has been something brought up at least a couple of times now and it ends up with the same answer from the community. [1] - https://review.openstack.org/#/c/107693/14 [2] - http://eavesdrop.openstack.org/meetings/cinder/2014/cinder.2014-10-15-16.00.log.html#l-186 [3] - https://wiki.openstack.org/wiki/CinderMeetings -- Mike Perez ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [cinder] [driver] DB operations
On 06:05 Sat 20 Dec , Duncan Thomas wrote: No, I mean that if drivers are going to access database, then they should do it via a defined interface that limits what they can do to a sane set of operations. I'd still prefer that they didn't need extra access beyond the model update, but I don't know if that is possible. Duncan Thomas On Dec 19, 2014 6:43 PM, Amit Das amit@cloudbyte.com wrote: Thanks Duncan. Do you mean hepler methods in the specific driver class? On 19 Dec 2014 14:51, Duncan Thomas duncan.tho...@gmail.com wrote: So our general advice has historical been 'drivers should not be accessing the db directly'. I haven't had chance to look at your driver code yet, I've been on vacation, but my suggestion is that if you absolutely must store something in the admin metadata rather than somewhere that is covered by the model update (generally provider location and provider auth) then writing some helper methods that wrap the context bump and db call would be better than accessing it directly from the driver. Duncan Thomas On Dec 18, 2014 11:41 PM, Amit Das amit@cloudbyte.com wrote: I've expressed in past reviews that we should have an interface that limits drivers access to the database [1], but received quite a bit of push back in Cinder. I recommend we stick to what has been decided, otherwise, Amit you should spend some time on reading the history of this issue [2] from previous meetings and start a rediscussion on it in the next meeting [3]. Not discouraging it, but this has been something brought up at least a couple of times now and it ends up with the same answer from the community. [1] - https://review.openstack.org/#/c/107693/14 [2] - http://eavesdrop.openstack.org/meetings/cinder/2014/cinder.2014-10-15-16.00.log.html#l-186 [3] - https://wiki.openstack.org/wiki/CinderMeetings -- Mike Perez ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [cinder] [driver] DB operations
No, I mean that if drivers are going to access database, then they should do it via a defined interface that limits what they can do to a sane set of operations. I'd still prefer that they didn't need extra access beyond the model update, but I don't know if that is possible. Duncan Thomas On Dec 19, 2014 6:43 PM, Amit Das amit@cloudbyte.com wrote: Thanks Duncan. Do you mean hepler methods in the specific driver class? On 19 Dec 2014 14:51, Duncan Thomas duncan.tho...@gmail.com wrote: So our general advice has historical been 'drivers should not be accessing the db directly'. I haven't had chance to look at your driver code yet, I've been on vacation, but my suggestion is that if you absolutely must store something in the admin metadata rather than somewhere that is covered by the model update (generally provider location and provider auth) then writing some helper methods that wrap the context bump and db call would be better than accessing it directly from the driver. Duncan Thomas On Dec 18, 2014 11:41 PM, Amit Das amit@cloudbyte.com wrote: Hi Stackers, I have been developing a Cinder driver for CloudByte storage and have come across some scenarios where the driver needs to do create, read update operations on cinder database (volume_admin_metadata table). This is required to establish a mapping between OpenStack IDs with the backend storage IDs. Now, I have got some review comments w.r.t the usage of DB related operations esp. w.r.t raising the context to admin. In short, it has been advised not to use *context.get_admin_context()* . https://review.openstack.org/#/c/102511/15/cinder/volume/drivers/cloudbyte/cloudbyte.py However, i get errors trying to use the default context as shown below: *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher File /opt/stack/cinder/cinder/db/sqlalchemy/api.py, line 103, in is_admin_context* *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher return context.is_admin* *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher AttributeError: 'module' object has no attribute 'is_admin'* So what is the proper way to run these DB operations from within a driver ? Regards, Amit *CloudByte Inc.* http://www.cloudbyte.com/ ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [cinder] [driver] DB operations
Got it Duncan. I will re-check if I can arrive at any solution without accessing the database. Regards, Amit *CloudByte Inc.* http://www.cloudbyte.com/ On Sat, Dec 20, 2014 at 7:35 PM, Duncan Thomas duncan.tho...@gmail.com wrote: No, I mean that if drivers are going to access database, then they should do it via a defined interface that limits what they can do to a sane set of operations. I'd still prefer that they didn't need extra access beyond the model update, but I don't know if that is possible. Duncan Thomas On Dec 19, 2014 6:43 PM, Amit Das amit@cloudbyte.com wrote: Thanks Duncan. Do you mean hepler methods in the specific driver class? On 19 Dec 2014 14:51, Duncan Thomas duncan.tho...@gmail.com wrote: So our general advice has historical been 'drivers should not be accessing the db directly'. I haven't had chance to look at your driver code yet, I've been on vacation, but my suggestion is that if you absolutely must store something in the admin metadata rather than somewhere that is covered by the model update (generally provider location and provider auth) then writing some helper methods that wrap the context bump and db call would be better than accessing it directly from the driver. Duncan Thomas On Dec 18, 2014 11:41 PM, Amit Das amit@cloudbyte.com wrote: Hi Stackers, I have been developing a Cinder driver for CloudByte storage and have come across some scenarios where the driver needs to do create, read update operations on cinder database (volume_admin_metadata table). This is required to establish a mapping between OpenStack IDs with the backend storage IDs. Now, I have got some review comments w.r.t the usage of DB related operations esp. w.r.t raising the context to admin. In short, it has been advised not to use *context.get_admin_context()* . https://review.openstack.org/#/c/102511/15/cinder/volume/drivers/cloudbyte/cloudbyte.py However, i get errors trying to use the default context as shown below: *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher File /opt/stack/cinder/cinder/db/sqlalchemy/api.py, line 103, in is_admin_context* *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher return context.is_admin* *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher AttributeError: 'module' object has no attribute 'is_admin'* So what is the proper way to run these DB operations from within a driver ? Regards, Amit *CloudByte Inc.* http://www.cloudbyte.com/ ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [cinder] [driver] DB operations
So our general advice has historical been 'drivers should not be accessing the db directly'. I haven't had chance to look at your driver code yet, I've been on vacation, but my suggestion is that if you absolutely must store something in the admin metadata rather than somewhere that is covered by the model update (generally provider location and provider auth) then writing some helper methods that wrap the context bump and db call would be better than accessing it directly from the driver. Duncan Thomas On Dec 18, 2014 11:41 PM, Amit Das amit@cloudbyte.com wrote: Hi Stackers, I have been developing a Cinder driver for CloudByte storage and have come across some scenarios where the driver needs to do create, read update operations on cinder database (volume_admin_metadata table). This is required to establish a mapping between OpenStack IDs with the backend storage IDs. Now, I have got some review comments w.r.t the usage of DB related operations esp. w.r.t raising the context to admin. In short, it has been advised not to use *context.get_admin_context()*. https://review.openstack.org/#/c/102511/15/cinder/volume/drivers/cloudbyte/cloudbyte.py However, i get errors trying to use the default context as shown below: *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher File /opt/stack/cinder/cinder/db/sqlalchemy/api.py, line 103, in is_admin_context* *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher return context.is_admin* *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher AttributeError: 'module' object has no attribute 'is_admin'* So what is the proper way to run these DB operations from within a driver ? Regards, Amit *CloudByte Inc.* http://www.cloudbyte.com/ ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [cinder] [driver] DB operations
On 01:20 Fri 19 Dec , Duncan Thomas wrote: So our general advice has historical been 'drivers should not be accessing the db directly'. I haven't had chance to look at your driver code yet, I've been on vacation, but my suggestion is that if you absolutely must store something in the admin metadata rather than somewhere that is covered by the model update (generally provider location and provider auth) then writing some helper methods that wrap the context bump and db call would be better than accessing it directly from the driver. Duncan Thomas On Dec 18, 2014 11:41 PM, Amit Das amit@cloudbyte.com wrote: snip So what is the proper way to run these DB operations from within a driver ? Drivers not doing db changes is also documented in the How to contribute a driver wiki page. https://wiki.openstack.org/wiki/Cinder/how-to-contribute-a-driver -- Mike Perez ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [cinder] [driver] DB operations
Thanks Duncan. Do you mean hepler methods in the specific driver class? On 19 Dec 2014 14:51, Duncan Thomas duncan.tho...@gmail.com wrote: So our general advice has historical been 'drivers should not be accessing the db directly'. I haven't had chance to look at your driver code yet, I've been on vacation, but my suggestion is that if you absolutely must store something in the admin metadata rather than somewhere that is covered by the model update (generally provider location and provider auth) then writing some helper methods that wrap the context bump and db call would be better than accessing it directly from the driver. Duncan Thomas On Dec 18, 2014 11:41 PM, Amit Das amit@cloudbyte.com wrote: Hi Stackers, I have been developing a Cinder driver for CloudByte storage and have come across some scenarios where the driver needs to do create, read update operations on cinder database (volume_admin_metadata table). This is required to establish a mapping between OpenStack IDs with the backend storage IDs. Now, I have got some review comments w.r.t the usage of DB related operations esp. w.r.t raising the context to admin. In short, it has been advised not to use *context.get_admin_context()*. https://review.openstack.org/#/c/102511/15/cinder/volume/drivers/cloudbyte/cloudbyte.py However, i get errors trying to use the default context as shown below: *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher File /opt/stack/cinder/cinder/db/sqlalchemy/api.py, line 103, in is_admin_context* *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher return context.is_admin* *2014-12-19 12:18:17.880 TRACE oslo.messaging.rpc.dispatcher AttributeError: 'module' object has no attribute 'is_admin'* So what is the proper way to run these DB operations from within a driver ? Regards, Amit *CloudByte Inc.* http://www.cloudbyte.com/ ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
