Hello Pete, Thank you for your review and sorry for very late response. > The most important in my mind is: why use rgw_user? You are completely right, rgw_user isn’t necessary for BNS. I preserved it because of type-correctness. Having separate language type for basic entities makes code easier to understand and extend in future. This trait could be particularly important if Yehuda decide to implement multi- tenancy. Those are the reasons I’m behind keeping rgw_user even if the entire information it carries would be solely an ID string. > I think, the information about a user should contain the user's bucket namespace, > but the user's label does not need to have it. So, RGWUserInfo should have the > bucket namespace name (and possibly has_own_bns) Yeah, bucket namespace is a part of account/RGWUserInfo*. Property “has_own_bns” even now is serialized together with RGWUserInfo. It’s passed as a part of rgw_user in order to skip reworking signatures of many functions and methods. However, I would say that in the end it’s worth to perform such refactoring and thus have rgw_user cleaned. I will work on that. > Less importantly, I do not like the generosity with knobs. Me too, but sometimes introduction of configuration parameter is necessary to not enforce new behavior that may break backward compatibility. > The rgw_swift_create_account_with_bns shold go away with rgw_user. Option "rgw_swift_create_account_with_bns" is needed mostly due to integration with OpenStack (Keystone) when accounts* are automatically created at first access. Without the parameter you would lose ability to tell radosgw what is more important for you: compliance with Swift API or previous behavior that still may be useful in some cases. Creating massive amount of accounts by hand might not be an option here. > The rgw_swift_account_in_url should be possible to incorporate > in a compatible fashion (it does not add an extra next_tok()). According to "rgw_swift_account_in_url": I don’t see viable method for deducing whether two tokens in URL refer to 1) account and bucket or 2) bucket and object. Of course, we may apply some kind of heuristic like scanning the first token for auth prefix (eg. “AUTH_”, “KEY_”) but this would introduce limitations on bucket naming. > - Is it just me, or do encoding and decoding of RGWUserInfo do > not match? Decoding appears to make provision for wip-5073, > which we may not even need. I'm not sure whether I understood you correctly. Serialization of bufferlist is versioned and we provide code to support previous versions. However, in the case of removing "tenant" (if we decide it's desired at the moment), touching original commits of wip-5073 in order to simply avoid adding it might be better. > - The --own-bucket-namespace should not be a boolean, but the > namespace's name. We may switch to this behavior if we think there are some use cases which could be covered with such transition. From technical point of view this is not a problem. Best regards, Radoslaw Zarzynski * in the context of radosgw under term "account" I understand some parts of RGWUserInfo and bucket index. -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
