Skip to content

WIP: feat: add urllib4 fetch #5425

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open

WIP: feat: add urllib4 fetch #5425

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/lib/egg.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
type HttpClientRequestOptions, type HttpClientRequestURL, type HttpClientResponse,
type HttpClientOptions,
} from './core/httpclient.js';
import { FetchFactory } from 'urllib';
import { createLoggers } from './core/logger.js';
import { convertObject } from './core/utils.js';
import { BaseContextClass } from './core/base_context_class.js';
Expand Down Expand Up @@ -146,6 +147,8 @@ export class EggApplicationCore extends EggCore {
agent?: Agent;
application?: Application;
declare loader: EggApplicationLoader;
FetchFactory = FetchFactory;
Copy link

Choose a reason for hiding this comment

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

medium

The line FetchFactory = FetchFactory; creates a class property with the same name as the imported module. While this makes urllib.FetchFactory accessible via this.FetchFactory, it can be slightly confusing due to the identical naming. Consider if an alias (e.g., urllibFetchFactory) would improve clarity, or if FetchFactory needs to be a class property at all if only fetch and setClientOptions are used directly on the imported module.

fetch = FetchFactory.fetch;

/**
* @class
Expand Down Expand Up @@ -178,6 +181,7 @@ export class EggApplicationCore extends EggCore {
this.lifecycle.registerBeforeStart(async () => {
await this.load();
}, 'load files');
this.FetchFactory.setClientOptions({});
Copy link

Choose a reason for hiding this comment

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

medium

The setClientOptions({}) call initializes urllib's client options with an empty object. While this uses urllib's defaults, it's often beneficial to explicitly configure common options like timeout, maxSockets, or proxy settings that are relevant to the application's environment or performance requirements. Consider adding specific default options here, or a comment explaining why no custom options are needed.

}

/**
Expand Down
Loading