From 6e0943f91bbb7350eec8e20338befc08d9eeb9f0 Mon Sep 17 00:00:00 2001 From: Tommy Brunn Date: Thu, 5 May 2022 12:49:02 +0200 Subject: [PATCH] Log one-time warning when not specifying a partitioner This is to hopefully avoid people mistakenly using the wrong partitioner because they're relying on the default. This warning will be removed in a future version. --- src/index.js | 15 +++++++++++++++ src/utils/once.js | 10 ++++++++++ src/utils/once.spec.js | 14 ++++++++++++++ src/utils/websiteUrl.js | 6 +++++- src/utils/websiteUrl.spec.js | 13 +++++++------ 5 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 src/utils/once.js create mode 100644 src/utils/once.spec.js diff --git a/src/index.js b/src/index.js index 006844195..ce94c5136 100644 --- a/src/index.js +++ b/src/index.js @@ -11,6 +11,8 @@ const createConsumer = require('./consumer') const createAdmin = require('./admin') const ISOLATION_LEVEL = require('./protocol/isolationLevel') const defaultSocketFactory = require('./network/socketFactory') +const once = require('./utils/once') +const websiteUrl = require('./utils/websiteUrl') const PRIVATE = { CREATE_CLUSTER: Symbol('private:Kafka:createCluster'), @@ -20,6 +22,15 @@ const PRIVATE = { } const DEFAULT_METADATA_MAX_AGE = 300000 +const warnOfDefaultPartitioner = once(logger => { + if (process.env.KAFKAJS_NO_PARTITIONER_WARNING == null) { + logger.warn( + `KafkaJS v2.0.0 switched default partitioner. To retain the same partitioning behavior as in previous versions, create the producer with the option "createPartitioner: Partitioners.LegacyPartitioner". See ${websiteUrl( + '/docs/2.0.0/migration-guide-v2.0.0' + )} for details. Silence this warning by setting the environment variable "KAFKAJS_NO_PARTITIONER_WARNING=1"` + ) + } +}) module.exports = class Client { /** @@ -104,6 +115,10 @@ module.exports = class Client { instrumentationEmitter, }) + if (createPartitioner == null) { + warnOfDefaultPartitioner(this[PRIVATE.LOGGER]) + } + return createProducer({ retry: { ...this[PRIVATE.CLUSTER_RETRY], ...retry }, logger: this[PRIVATE.LOGGER], diff --git a/src/utils/once.js b/src/utils/once.js new file mode 100644 index 000000000..c29ba7ba8 --- /dev/null +++ b/src/utils/once.js @@ -0,0 +1,10 @@ +module.exports = fn => { + let called = false + + return (...args) => { + if (!called) { + called = true + return fn(...args) + } + } +} diff --git a/src/utils/once.spec.js b/src/utils/once.spec.js new file mode 100644 index 000000000..753b47063 --- /dev/null +++ b/src/utils/once.spec.js @@ -0,0 +1,14 @@ +const once = require('./once') + +describe('Utils > once', () => { + it('should call the wrapped function only once', () => { + const original = jest.fn().mockReturnValue('foo') + const wrapped = once(original) + + expect(wrapped('hello')).toEqual('foo') + expect(wrapped('hello')).toBeUndefined() + + expect(original).toHaveBeenCalledTimes(1) + expect(original).toHaveBeenCalledWith('hello') + }) +}) diff --git a/src/utils/websiteUrl.js b/src/utils/websiteUrl.js index 0e721d7b2..cb3744247 100644 --- a/src/utils/websiteUrl.js +++ b/src/utils/websiteUrl.js @@ -1,3 +1,7 @@ const BASE_URL = 'https://kafka.js.org' +const stripLeading = char => str => (str.charAt(0) === char ? str.substring(1) : str) +const stripLeadingSlash = stripLeading('/') +const stripLeadingHash = stripLeading('#') -module.exports = (path, hash) => `${BASE_URL}/${path}${hash ? '#' + hash : ''}` +module.exports = (path, hash) => + `${BASE_URL}/${stripLeadingSlash(path)}${hash ? '#' + stripLeadingHash(hash) : ''}` diff --git a/src/utils/websiteUrl.spec.js b/src/utils/websiteUrl.spec.js index 5dc0d8549..3eb461580 100644 --- a/src/utils/websiteUrl.spec.js +++ b/src/utils/websiteUrl.spec.js @@ -1,14 +1,15 @@ const websiteUrl = require('./websiteUrl') describe('Utils > websiteUrl', () => { - it('generates links to the website', () => { - expect(websiteUrl('docs/faq')).toEqual('https://kafka.js.org/docs/faq') + it.each([['docs/faq'], ['/docs/faq']])('generates links to the website', path => { + expect(websiteUrl(path)).toEqual('https://kafka.js.org/docs/faq') }) - it('allows specifying a hash', () => { - expect( - websiteUrl('docs/faq', 'why-am-i-receiving-messages-for-topics-i-m-not-subscribed-to') - ).toEqual( + it.each([ + ['why-am-i-receiving-messages-for-topics-i-m-not-subscribed-to'], + ['#why-am-i-receiving-messages-for-topics-i-m-not-subscribed-to'], + ])('allows specifying an anchor', anchor => { + expect(websiteUrl('docs/faq', anchor)).toEqual( 'https://kafka.js.org/docs/faq#why-am-i-receiving-messages-for-topics-i-m-not-subscribed-to' ) })