Addressing the following Redis related issues.

[ehardy]

Problem

Redis seems to have become mandatory in recent releases. We have noticed a couple of issues after grabbing the latest version.

1. Failure to initialize the RedisTransport object in src/logger.ts in cases where Redis requires password authentication (which is the case in our environments).
2. It did not seem like Redis was getting initialized for the persisted query feature. It appeared that way as we were noticing Redis related warnings in the logs when exercising the persisted query feature.
3. Streaming registry logs over Redis should be an opt in feature in our opinions, so simply checking if a Redis host is configured before enabling the feature is not enough.

Changes

• Passing the Redis password to the RedisTransport constructor in src/logger.ts.
• Initializing Redis in the init() function in src/index.ts. With this change the warnings in the logs disappear.
• In order to cover the Redis/password cases, the docker-compose.base.yml now makes sure to launch Redis with password protection.
• Adding an environment variable called LOG_STREAMING_ENABLED to further control if log streaming over Redis should be enabled or not.

Testing

All existing unit and integration tests are passing.

[pipedrive-bot-eventsink]

Main branch dependencies

[ehardy]

Will have a look at the failing checks

[ehardy]

@tot-ra All checks are now green. Could you have a look at the PR?

[tot-ra]

I'd suggest to enable it by default to have richest experience by default

[tot-ra]

Or what do you think.. should we move to most minimal version by default (no redis, no kafka.. maybe even no mysql -> sqlite?)

[ehardy]

I'd suggest to enable it by default to have richest experience by default

Sounds reasonable to me. As long as there's a configuration to disable it, I don't mind having to disable it within our deployments. I'll invert the logic.

[tot-ra]

also noticed that by default if there are no logs, then UI doesn't handle it well.. If you can handle that, great. If not, I can fix in follow-up

[ehardy]

also noticed that by default if there are no logs, then UI doesn't handle it well.. If you can handle that, great. If not, I can fix in follow-up

We had implemented this logic within our fork and it seemed to work well. I'll check it out.

[ehardy]

@tot-ra Everything should be good now. Streaming logs over Redis is enabled by default. Could you have another look?

[tot-ra]

hmm.. I keep getting

graphql-schema-registry-gql-schema-registry-1            | [2022-12-02T02:21:07.194Z] error: Redis error NOAUTH Authentication required. ReplyError: NOAUTH Authentication required.
graphql-schema-registry-gql-schema-registry-1            |     at Object.onceWrapper (node:events:627:28)
graphql-schema-registry-gql-schema-registry-1            |     at Socket.emit (node:events:525:35)

when I run docker-compose -f docker-compose.base.yml -f docker-compose.dev.yml up

[ehardy]

@tot-ra Fixed the docker-compose.dev.yml configs. I never use this docker compose file, my bad. Should the docker-compose.prod.yml file be adjusted too?

[ehardy]

Hey @tot-ra , would you be able to take another look at this?

[tot-ra]

@ehardy hey. Unfortunately I don't have access to pipedrive namespace anymore. I'm waiting for Pipedrive legal, devops tooling, core services team and CTO to handover repo to my personal namespace. If that doesn't happen until end of this year, I guess I'll fork it and continue work separately from this namespace

[rdepping]

@ehardy hey. Unfortunately I don't have access to pipedrive namespace anymore. I'm waiting for Pipedrive legal, devops tooling, core services team and CTO to handover repo to my personal namespace. If that doesn't happen until end of this year, I guess I'll fork it and continue work separately from this namespace

@tot-ra Do you have any update on the future direction and support for the pipedrive schema registry?

[tot-ra]

@rdepping I've forked the repo to my namespace - https://github.com/tot-ra/graphql-schema-registry hopefully I can maintain it instead