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

Underscores disappeared for y -> ies plurals #376

Closed
cben opened this issue Dec 12, 2018 · 3 comments
Closed

Underscores disappeared for y -> ies plurals #376

cben opened this issue Dec 12, 2018 · 3 comments
Labels

Comments

@cben
Copy link
Collaborator

cben commented Dec 12, 2018

4.1.0 includes #366 that broke method names for plurals that are not strict suffix of the singular kind.

Differences between method names in kubeclient 4.0.0 to 4.1.0, across multiple kubernetes & openshift versions:
https://gist.github.com/cben/344a0c7bf74ea50f185c29b52fe0678f#file-changes-diff

  • network_policy network_policies became networkpolicy networkpolicies.
  • pod_security_policy pod_security_policies became podsecuritypolicy podsecuritypolicies.
  • egress_network_policy egress_network_policies became egressnetworkpolicy egressnetworkpolicies.
  • cluster_policy cluster_policies became clusterpolicy clusterpolicies.
  • network_policy network_policies became networkpolicy networkpolicies.

it's not in principle limited to y -> ies plurals, e.g. if you had a CRD with BigDatum / bigdata then methods were big_datum big_data previously and became bigdatum bigdata in 4.1.0.

I want to fix the above ASAP in 4.1.1, restoring the underscores.


Other change that I'd say is a bug sorry meant isn't a bug:
Entities where kind is inconsistent with plural name previously were skipped — no methods were defined for them:

  • ReplicationControllerDummy replicationcontrollers (extensions/v1beta1)
  • DeploymentConfig generatedeploymentconfigs (oapi/v1)
  • Template processedtemplates (template.openshift.io/v1, oapi/v1)

Now these always get "fallback" methods defined — using kind.downcase and name, with no underscores attempted.
This change I kinda intended. cc @masayag — you're tweaking this fallback in #373 — do you agree this last fallback is good, or is there a reason we revert these as well in 4.1.1?

@masayag
Copy link
Contributor

masayag commented Dec 12, 2018

This change I kinda intended. cc @masayag — you're tweaking this fallback in #373 — do you agree this last fallback is good, or is there a reason we revert these as well in 4.1.1?

@cben I think the tweaked fallback is good. It makes more sense to me to have
(replicationcontrollerdummy, replicationcontrollers) than having (replication_controller_dummy, replicationcontrollers) in terms of predictability.

@cben cben added the bug label Dec 13, 2018
cben added a commit to cben/kubeclient that referenced this issue Dec 15, 2018
cben added a commit to cben/kubeclient that referenced this issue Dec 15, 2018
cben added a commit to cben/kubeclient that referenced this issue Dec 15, 2018
cben added a commit to cben/kubeclient that referenced this issue Dec 15, 2018
@cben
Copy link
Collaborator Author

cben commented Dec 16, 2018

The plot thickens (out-of-scope for this issue — this one is just about disappeared methods — but continuing here as we're already discussing this):
Of the 3 methods now added thanks to the fallback, "ReplicationControllerDummy" is silly and obsolete, and the other 2 collide with other resources with same kind:

      "name": "templates",
      "kind": "Template",
      "verbs": ["create", "delete", ...]
...
      "name": "processedtemplates",
      "kind": "Template",
      "verbs": ["create"]

(both oapi/v1 as well as apis/template.openshift.io/v1 contain this collision)

=> opened openshift/origin#21668 to get clarification on this collision.

Previous behaviour ignored processedtemplates — no methods were created for this.
New behaviour would try creating methods for both:

	template	processedtemplates
	template	templates

with last one arbitrarily winning for get_template :-(

Up to openshift v3.6, it also had a 2nd kind collision:

      "name": "generatedeploymentconfigs",
      "kind": "DeploymentConfig",
      "verbs": []
...
      "name": "deploymentconfigs",
      "kind": "DeploymentConfig",
      "verbs": ["create", "delete", ...]

Amusingly, this one does not result in method collision, because only the former gets underscores:

	deployment_config	deployment_configs	
	deploymentconfig	generatedeploymentconfigs	

In theory, the generic fallback is useful for CRDs, but looking at these examples it's mostly harmful 😒

Perhaps cleanest solution is special-casing processedtemplates to processed_template processed_templates.

@cben
Copy link
Collaborator Author

cben commented Dec 17, 2018

OK, processedtemplates endpoint doesn't need the usual get_, create_ etc methods — it only supports POST, behaves specially — takes template + params returns result of substituting params — and is already covered by special process_template method of kubeclient.
I'll find a way to blacklist it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants