oneclickexport: add code host configs export functionality #39855
Conversation
| return s.Type | ||
| } | ||
|
|
||
| type CodeHostConfigProcessor struct { |
There was a problem hiding this comment.
Would it make sense to split these processor implementation types into different files? If we add just a few more, it might start to get cluttered here.
There was a problem hiding this comment.
I think we should do it when it starts to become cluttered. Too many files too early on does the opposite right now.
There was a problem hiding this comment.
I am planning to separate the files by type (config export, db query export, etc) when there is more code, as Indra mentioned
| func (c CodeHostConfigProcessor) Process(ctx context.Context, _ *ConfigRequest, dir string) { | ||
| externalServices, err := c.db.ExternalServices().List(ctx, database.ExternalServicesListOptions{}) | ||
| if err != nil { | ||
| c.logger.Error("Error during getting external services", log.Error(err)) |
There was a problem hiding this comment.
error getting external services
| c.logger.Error("Error during marshalling the code host config", log.Error(err)) | ||
| } | ||
|
|
||
| err = ioutil.WriteFile(dir+"/code-host-config.txt", configBytes, 0644) |
There was a problem hiding this comment.
Similar to my comment on the other PR, is there a reason for the .txt over .json?
There was a problem hiding this comment.
changed to .json
| err = ioutil.WriteFile(dir+"/code-host-config.txt", configBytes, 0644) | ||
|
|
||
| if err != nil { | ||
| c.logger.Error("Error during site config export", log.Error(err)) |
There was a problem hiding this comment.
error during code host config export
| const ( | ||
| wantSiteConfig = `{ |
There was a problem hiding this comment.
(Not sure why I can't see the suggest changes button in this PR)
Nit: I see only a single const here. Maybe we can declare it directly const wantSiteConfig = ... instead of const ()?
There was a problem hiding this comment.
they're added in following PR :)
| return s.Type | ||
| } | ||
|
|
||
| type CodeHostConfigProcessor struct { |
There was a problem hiding this comment.
I think we should do it when it starts to become cluttered. Too many files too early on does the opposite right now.
sashaostrikov
force-pushed
the
ao/one-click-export-code-host-config
branch
from
10af0b9 to
3a82fc8
Compare
One thing: current design of how
ExportRequestis composed and processed (very ugly and straightforward ATM) will be changed when newProcessortypes are added.I'll resolve merge conflicts after parent PR is merged.✅Conflicts resolved✅
Depends on https://github.com/sourcegraph/sourcegraph/pull/39813
Closes https://github.com/sourcegraph/sourcegraph/issues/39618
Test plan
Separate test case for code host config export is added. Cumulative test for complete export of everything currently supported is added.