Skip to content
This repository was archived by the owner on Aug 14, 2024. It is now read-only.

Conversation

@goganchic
Copy link
Contributor

@goganchic goganchic commented Jul 3, 2022

necessary code changes: getsentry/self-hosted#1560

@vercel
Copy link

vercel bot commented Jul 3, 2022

@goganchic is attempting to deploy a commit to the Sentry Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@aminvakil aminvakil left a comment

Choose a reason for hiding this comment

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

Thank you for keeping to improve docs!

"httpProxy": "http://proxy:3128",
"httpsProxy": "http://proxy:3128",
"noProxy": "127.0.0.0/8"
"noProxy": "relay,web,sentry,127.0.0.0/8"
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested a clean working installation with these variables? Are these enough? We have kafka:9092 in relay/config.yml for example, shouldn't we also add kafka to noProxy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd removed all docker containers, images, volumes and networks and reinstalled Sentry one more time, checked web, created test event. I'd found no errors in docker logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please wait a bit, I need to recheck it one more time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the one hand proxies from ~/.docker/config.json work only for docker run command and do not work for docker build. It is written in the docker docs:

When you create or start new containers, the environment variables are set automatically within the container.

From the other hand we need a connection to the Internet only during build stage. So we do not need ~/.docker/config.json at all. We need to set correct env variables through /etc/environment and that's it. In this commit getsentry/self-hosted@2672ce5 we do not need changes in /etc/apt/apt.conf, because all required variables will be passed through /etc/environment.

Sorry for my mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aminvakil I'd updated PR, please take a look.

@vercel
Copy link

vercel bot commented Jul 5, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
develop ✅ Ready (Inspect) Visit Preview Jul 15, 2022 at 10:13PM (UTC)

@chadwhitacre
Copy link
Member

Catching up on the threads ... what's our status here? getsentry/self-hosted#1560 is closed, do we still want to remove step (4) of the docs with this PR?

@goganchic
Copy link
Contributor Author

Yes. Before getsentry/self-hosted#1560 I thought that step 1 is redundant, but actually it's required and step 4 is not.

@goganchic
Copy link
Contributor Author

Actually, step 4 is required if you want to send daily stats to sentry.io with SENTRY_BEACON. Should I add such comment?

@chadwhitacre
Copy link
Member

I had the same thought. Not everyone will care to do that but it would be good to note in a comment, yes.

Thanks! :)

@goganchic
Copy link
Contributor Author

@chadwhitacre I've added the information about the configuration access for Sentry Beacon.

Copy link
Member

@chadwhitacre chadwhitacre left a comment

Choose a reason for hiding this comment

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

Thanks for the attention to detail, @goganchic!

Copy link
Contributor

@aminvakil aminvakil left a comment

Choose a reason for hiding this comment

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

LGTM!

@goganchic Thanks for improving docs!

@chadwhitacre chadwhitacre enabled auto-merge (squash) July 15, 2022 22:11
@chadwhitacre chadwhitacre merged commit 5dbae5e into getsentry:master Jul 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants