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

add default_namespaced - for #209 #413

Closed
wants to merge 1 commit into from
Closed

add default_namespaced - for #209 #413

wants to merge 1 commit into from

Conversation

clux
Copy link
Member

@clux clux commented Feb 11, 2021

based on suggestion in #394 (comment)

WIP. POC there, but missing some tests. And probably need to test through Api, which makes this a completely new test path.

@clux clux linked an issue Feb 11, 2021 that may be closed by this pull request
@clux
Copy link
Member Author

clux commented Feb 11, 2021

Not sure I need to do it this way. I have the Client, and it should have the default_ns set at this point. Could possibly just do #209 (comment)
EDIT: no i am tired.

kube/src/api/typed.rs Outdated Show resolved Hide resolved
kube/src/service/mod.rs Outdated Show resolved Hide resolved
@clux clux force-pushed the default_namespaced branch from 8b150e1 to 5457577 Compare February 13, 2021 17:21
@clux
Copy link
Member Author

clux commented Feb 13, 2021

Going to wait with this a bit. It's awkwardly coupling Api and Service in a way that requires an entirely new way to test.

I think having a way to test the Client would be useful, but am going to explore that before merging this so we can have some tests on it.

#[test]
fn api_default_ns() {
    use k8s_openapi::api::core::v1::Pod;
    let client = Client::mock();
    let pods: Api<Pod> = Api::default_namespaced(&client);
    let _p = api.get("blod").await?
    // TODO: intercept outgoing call and inspect request elements

    // TODO: verify request url has updated the default namespace (set to non-default in Client::mock)
}

It's possible it's necessary to copy default_ns into Client so that Api::default_namespaced can use a getter on the Client instead. Not elegant, but it gets rid of the coupling.

Anyway going to experiment a bit with mocks first.

@kazk kazk mentioned this pull request May 21, 2021
@clux
Copy link
Member Author

clux commented May 21, 2021

Replaced by #534

@clux clux closed this May 21, 2021
@clux clux deleted the default_namespaced branch May 21, 2021 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Api::namespaced should work with default namespace
2 participants