Skip to content

Add TQ support #1002

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

Merged
merged 3 commits into from
Nov 11, 2021
Merged

Add TQ support #1002

merged 3 commits into from
Nov 11, 2021

Conversation

inlined
Copy link
Member

@inlined inlined commented Nov 5, 2021

Adds preview support for TQ.

V1 APIs should have an @hidden annotation. V2 APIs area already hidden behind obscure imports.

I'm having some trouble with code complete not prompting unless I start typing. If anyone knows what the issue might be there I'd love to know.

@inlined inlined requested a review from taeold November 5, 2021 14:32
@google-cla google-cla bot added the cla: yes label Nov 5, 2021
@@ -172,35 +220,6 @@ function checkAppCheckContext(
expect(context.instanceIdToken).to.be.undefined;
}

function checkAuthRequest(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted these because a CallableRequest is a CallableContext. Made the overload explicit in the method I kept anyway.

@@ -648,6 +667,187 @@ describe('onCallHandler', () => {
});
});

describe('onEnqueueHandler', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy + paste + tweak from onCallHandler

@@ -201,6 +208,106 @@ describe('#onCall', () => {
});
});

describe('#onEnqueue', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy + paste + tweak from onCall

@@ -366,3 +366,139 @@ describe('onCall', () => {
https.onCall((request: https.CallableRequest) => `Hello, ${request.data}`);
});
});

describe('onTaskEnqueue', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy + paste + tweak from onCall

Copy link
Contributor

@taeold taeold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, few suggested fixes.

@@ -169,6 +169,56 @@ export interface CallableRequest<T = any> {
rawRequest: Request;
}

/** How a task should be retried in the event of a non-2xx return. */
export interface TaskRetryConfig {
// If left unspecified, will default to 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Should be leave this comment out? Wondering if this comment is better left out since it's the responsibility of the CLI/Cloud Task Queue to set the defaults if missing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the bug is that these aren't customer docs.

@inlined inlined merged commit 38f1946 into master Nov 11, 2021
@inlined inlined deleted the inlined.tq branch November 12, 2021 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants