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

feat/sg: prototype 'sg start sourcegraph-accounts'#63894

Merged
bobheadxi merged 2 commits into
mainfrom
sg-config-sams-prototype
Jul 19, 2024
Merged

feat/sg: prototype 'sg start sourcegraph-accounts'#63894
bobheadxi merged 2 commits into
mainfrom
sg-config-sams-prototype

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Jul 17, 2024

Copy link
Copy Markdown
Member

Prototype what it might be like to have sg run sourcegraph-accounts. I think this would be a huge improvement over the multi-step, many-things-to-install process that currently exists for SAMS.

There are a couple of quirks, but I think I'll leave this here for now, as this is part https://linear.app/sourcegraph/issue/CORE-220, a timeboxed spike into polishing some local-dev DX for SAMS.

Also see https://github.com/sourcegraph/sourcegraph-accounts/pull/262

Test plan

sg start sourcegraph-accounts

Go to 127.0.0.1:9991 and try to log in

@cla-bot cla-bot Bot added the cla-signed label Jul 17, 2024
@bobheadxi bobheadxi requested review from a team and eseliger July 17, 2024 23:18

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yay!!!!

Comment thread sg.config.yaml
expected to be configured in '../sourcegraph-accounts'
install: |
createdb -h $PGHOST -p $PGPORT -U $PGUSER --encoding=UTF8 --template=template0 accounts || true
cd ../sourcegraph-accounts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we be slightly more helpful here perhaps and do a if [ ! -d ../sgaccs ]; then echo "You need to clone that repo to <full path>"; fi ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added!

Comment thread sg.config.yaml
cd ../sourcegraph-accounts
./.bin/accounts-server
watch:
- ../sourcegraph-accounts/go.mod

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh wow haha I did not expect that would work

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, crossed my fingers and it just worked 😁

(though I think best practice wouldve dictated we check that each path is in cwd, but I'm not complaining 😆 )

Comment thread sg.config.yaml
Comment on lines +1160 to +1162
GOOGLE_CLIENT_SECRET:
project: sourcegraph-local-dev
name: SAMS_LOCAL_DEV_GOOGLE_CLIENT_SECRET

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤩

Comment thread sg.config.yaml Outdated
Comment thread sg.config.yaml Outdated
# use this port as the redirect URL, so we just stick to it for now.
PORT: 9991
NOTIFICATION_GCP_PUBSUB_PROJECT: sourcegraph-local-dev # used by gcp-pubsub-emulator
NOTIFICATION_GCP_PUBSUB_TOPIC: notifications

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we then globally set PUBSUB_EMULATOR_HOST: 'localhost:8043' so sg services actually use the emulator?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, I just removed this, since I gave up on the emulator for now. Instead, the default dumps the would-be-published payloads to logs

Comment thread sg.config.yaml Outdated
Comment thread sg.config.yaml
@bobheadxi bobheadxi merged commit f53e211 into main Jul 19, 2024
@bobheadxi bobheadxi deleted the sg-config-sams-prototype branch July 19, 2024 20:30

@unknwon unknwon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants