diff options
Diffstat (limited to 'doc/development')
23 files changed, 642 insertions, 190 deletions
diff --git a/doc/development/README.md b/doc/development/README.md index 0cafc112b6b..6892838be7f 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -22,6 +22,7 @@ comments: false - [UX guide](ux_guide/index.md) for building GitLab with existing CSS styles and elements - [Frontend guidelines](fe_guide/index.md) +- [Emoji guide](fe_guide/emojis.md) ## Backend guides @@ -37,6 +38,7 @@ comments: false - [Gotchas](gotchas.md) to avoid - [Issue and merge requests state models](object_state_models.md) - [How to dump production data to staging](db_dump.md) +- [Working with the GitHub importer](github_importer.md) ## Performance guides @@ -69,6 +71,7 @@ comments: false - [Iterating tables in batches](iterating_tables_in_batches.md) - [Ordering table columns](ordering_table_columns.md) - [Verifying database capabilities](verifying_database_capabilities.md) +- [Database Debugging and Troubleshooting](database_debugging.md) ## Testing guides diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index 5452b0e7a2f..fd2b9d0e908 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -68,10 +68,10 @@ BackgroundMigrationWorker.perform_async('BackgroundMigrationClassName', [arg1, a ``` Usually it's better to enqueue jobs in bulk, for this you can use -`BackgroundMigrationWorker.perform_bulk`: +`BackgroundMigrationWorker.bulk_perform_async`: ```ruby -BackgroundMigrationWorker.perform_bulk( +BackgroundMigrationWorker.bulk_perform_async( [['BackgroundMigrationClassName', [1]], ['BackgroundMigrationClassName', [2]]] ) @@ -85,13 +85,13 @@ updates. Removals in turn can be handled by simply defining foreign keys with cascading deletes. If you would like to schedule jobs in bulk with a delay, you can use -`BackgroundMigrationWorker.perform_bulk_in`: +`BackgroundMigrationWorker.bulk_perform_in`: ```ruby jobs = [['BackgroundMigrationClassName', [1]], ['BackgroundMigrationClassName', [2]]] -BackgroundMigrationWorker.perform_bulk_in(5.minutes, jobs) +BackgroundMigrationWorker.bulk_perform_in(5.minutes, jobs) ``` ## Cleaning Up @@ -201,7 +201,7 @@ class ScheduleExtractServicesUrl < ActiveRecord::Migration ['ExtractServicesUrl', [id]] end - BackgroundMigrationWorker.perform_bulk(jobs) + BackgroundMigrationWorker.bulk_perform_async(jobs) end end diff --git a/doc/development/changelog.md b/doc/development/changelog.md index f869938fe11..48cffc0dd18 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -80,7 +80,7 @@ changes. The first example focuses on _how_ we fixed something, not on _what_ it fixes. The rewritten version clearly describes the _end benefit_ to the user (fewer 500 -errors), and _when_ (searching commits with ElasticSearch). +errors), and _when_ (searching commits with Elasticsearch). Use your best judgement and try to put yourself in the mindset of someone reading the compiled changelog. Does this entry add value? Does it offer context diff --git a/doc/development/database_debugging.md b/doc/development/database_debugging.md new file mode 100644 index 00000000000..50eb8005b44 --- /dev/null +++ b/doc/development/database_debugging.md @@ -0,0 +1,55 @@ +# Database Debugging and Troubleshooting + +This section is to help give some copy-pasta you can use as a reference when you +run into some head-banging database problems. + +An easy first step is to search for your error in Slack or google "GitLab <my error>". + +--- + +Available `RAILS_ENV` + + - `production` (generally not for your main GDK db, but you may need this for e.g. omnibus) + - `development` (this is your main GDK db) + - `test` (used for tests like rspec and spinach) + + +## Nuke everything and start over + +If you just want to delete everything and start over with an empty DB (~1 minute): + + - `bundle exec rake db:reset RAILS_ENV=development` + +If you just want to delete everything and start over with dummy data (~40 minutes). This also does `db:reset` and runs DB-specific migrations: + + - `bundle exec rake dev:setup RAILS_ENV=development` + +If your test DB is giving you problems, it is safe to nuke it because it doesn't contain important data: + + - `bundle exec rake db:reset RAILS_ENV=test` + +## Migration wrangling + + - `bundle exec rake db:migrate RAILS_ENV=development`: Execute any pending migrations that you may have picked up from a MR + - `bundle exec rake db:migrate:status RAILS_ENV=development`: Check if all migrations are `up` or `down` + - `bundle exec rake db:migrate:down VERSION=20170926203418 RAILS_ENV=development`: Tear down a migration + - `bundle exec rake db:migrate:up VERSION=20170926203418 RAILS_ENV=development`: Setup a migration + - `bundle exec rake db:migrate:redo VERSION=20170926203418 RAILS_ENV=development`: Re-run a specific migration + + +## Manually access the database + +Access the database via one of these commands (they all get you to the same place) + +``` +gdk psql -d gitlabhq_development +bundle exec rails dbconsole RAILS_ENV=development +bundle exec rails db RAILS_ENV=development +``` + + - `\q`: Quit/exit + - `\dt`: List all tables + - `\d+ issues`: List columns for `issues` table + - `CREATE TABLE board_labels();`: Create a table called `board_labels` + - `SELECT * FROM schema_migrations WHERE version = '20170926203418';`: Check if a migration was run + - `DELETE FROM schema_migrations WHERE version = '20170926203418';`: Manually remove a migration diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md index 0e4ffbd7910..db13e0e6249 100644 --- a/doc/development/doc_styleguide.md +++ b/doc/development/doc_styleguide.md @@ -170,12 +170,6 @@ You can combine one or more of the following: = link_to 'Help page', help_page_path('user/permissions'), class: 'btn btn-info' ``` -1. **Underlining a link.** - - ```haml - = link_to 'Help page', help_page_path('user/permissions'), class: 'underlined-link' - ``` - 1. **Using links inline of some text.** ```haml @@ -303,10 +297,10 @@ GitLab.com or http://docs.gitlab.com. Make sure this is discussed with the Documentation team beforehand. If you indeed need to change a document's location, do NOT remove the old -document, but rather put a text in it that points to the new location, like: +document, but rather replace all of its contents with a new line: ``` -This document was moved to [path/to/new_doc.md](path/to/new_doc.md). +This document was moved to [another location](path/to/new_doc.md). ``` where `path/to/new_doc.md` is the relative path to the root directory `doc/`. @@ -320,7 +314,7 @@ For example, if you were to move `doc/workflow/lfs/lfs_administration.md` to 1. Replace the contents of `doc/workflow/lfs/lfs_administration.md` with: ``` - This document was moved to [administration/lfs.md](../../administration/lfs.md). + This document was moved to [another location](../../administration/lfs.md). ``` 1. Find and replace any occurrences of the old location with the new one. diff --git a/doc/development/fe_guide/axios.md b/doc/development/fe_guide/axios.md new file mode 100644 index 00000000000..962fe3dcec9 --- /dev/null +++ b/doc/development/fe_guide/axios.md @@ -0,0 +1,68 @@ +# Axios +We use [axios][axios] to communicate with the server in Vue applications and most new code. + +In order to guarantee all defaults are set you *should not use `axios` directly*, you should import `axios` from `axios_utils`. + +## CSRF token +All our request require a CSRF token. +To guarantee this token is set, we are importing [axios][axios], setting the token, and exporting `axios` . + +This exported module should be used instead of directly using `axios` to ensure the token is set. + +## Usage +```javascript + import axios from '~/lib/utils/axios_utils'; + + axios.get(url) + .then((response) => { + // `data` is the response that was provided by the server + const data = response.data; + + // `headers` the headers that the server responded with + // All header names are lower cased + const paginationData = response.headers; + }) + .catch(() => { + //handle the error + }); +``` + +## Mock axios response on tests + +To help us mock the responses we need we use [axios-mock-adapter][axios-mock-adapter] + + +```javascript + import axios from '~/lib/utils/axios_utils'; + import MockAdapter from 'axios-mock-adapter'; + + let mock; + beforeEach(() => { + // This sets the mock adapter on the default instance + mock = new MockAdapter(axios); + // Mock any GET request to /users + // arguments for reply are (status, data, headers) + mock.onGet('/users').reply(200, { + users: [ + { id: 1, name: 'John Smith' } + ] + }); + }); + + afterEach(() => { + mock.reset(); + }); +``` + +### Mock poll requests on tests with axios + +Because polling function requires an header object, we need to always include an object as the third argument: + +```javascript + mock.onGet('/users').reply(200, { foo: 'bar' }, {}); +``` + +[axios]: https://github.com/axios/axios +[axios-instance]: #creating-an-instance +[axios-interceptors]: https://github.com/axios/axios#interceptors +[axios-mock-adapter]: https://github.com/ctimmerm/axios-mock-adapter diff --git a/doc/development/fe_guide/dropdowns.md b/doc/development/fe_guide/dropdowns.md new file mode 100644 index 00000000000..e1660ac5caa --- /dev/null +++ b/doc/development/fe_guide/dropdowns.md @@ -0,0 +1,38 @@ +# Dropdowns + + +## How to style a bootstrap dropdown +1. Use the HTML structure provided by the [docs][bootstrap-dropdowns] +1. Add a specific class to the top level `.dropdown` element + + + ```Haml + .dropdown.my-dropdown + %button{ type: 'button', data: { toggle: 'dropdown' }, 'aria-haspopup': true, 'aria-expanded': false } + %span.dropdown-toggle-text + Toggle Dropdown + = icon('chevron-down') + + %ul.dropdown-menu + %li + %a + item! + ``` + + Or use the helpers + ```Haml + .dropdown.my-dropdown + = dropdown_toggle('Toogle!', { toggle: 'dropdown' }) + = dropdown_content + %li + %a + item! + ``` + +1. Include the mixin in CSS + + ```SCSS + @include new-style-dropdown('.my-dropdown '); + ``` + +[bootstrap-dropdowns]: https://getbootstrap.com/docs/3.3/javascript/#dropdowns diff --git a/doc/development/fe_guide/emojis.md b/doc/development/fe_guide/emojis.md new file mode 100644 index 00000000000..38794c47965 --- /dev/null +++ b/doc/development/fe_guide/emojis.md @@ -0,0 +1,27 @@ +# Emojis + +GitLab supports native unicode emojis and fallsback to image-based emojis selectively +when your platform does not support it. + +# How to update Emojis + + 1. Update the `gemojione` gem + 1. Update `fixtures/emojis/index.json` from [Gemojione](https://github.com/jonathanwiesel/gemojione/blob/master/config/index.json). + In the future, we could grab the file directly from the gem. + We should probably make a PR on the Gemojione project to get access to + all emojis after being parsed or just a raw path to the `json` file itself. + 1. Ensure [`emoji-unicode-version`](https://www.npmjs.com/package/emoji-unicode-version) + is up to date with the latest version. + 1. Run `bundle exec rake gemojione:aliases` + 1. Run `bundle exec rake gemojione:digests` + 1. Run `bundle exec rake gemojione:sprite` + 1. Ensure new sprite sheets generated for 1x and 2x + - `app/assets/images/emoji.png` + - `app/assets/images/emoji@2x.png` + 1. Ensure you see new individual images copied into `app/assets/images/emoji/` + 1. Ensure you can see the new emojis and their aliases in the GFM Autocomplete + 1. Ensure you can see the new emojis and their aliases in the award emoji menu + 1. You might need to add new emoji unicode support checks and rules for platforms + that do not support a certain emoji and we need to fallback to an image. + See `app/assets/javascripts/emoji/support/is_emoji_unicode_supported.js` + and `app/assets/javascripts/emoji/support/unicode_support_map.js` diff --git a/doc/development/fe_guide/icons.md b/doc/development/fe_guide/icons.md index a76e978bd26..b288ee95722 100644 --- a/doc/development/fe_guide/icons.md +++ b/doc/development/fe_guide/icons.md @@ -4,15 +4,17 @@ We are using SVG Icons in GitLab with a SVG Sprite, due to this the icons are on ### Usage in HAML/Rails -To use a sprite Icon in HAML or Rails we use a specific helper function : +To use a sprite Icon in HAML or Rails we use a specific helper function : `sprite_icon(icon_name, size: nil, css_class: '')` -**icon_name** Use the icon_name that you can find in the SVG Sprite (Overview is available under `/assets/sprite.symbol.html`). +**icon_name** Use the icon_name that you can find in the SVG Sprite ([Overview is available here](http://gitlab-org.gitlab.io/gitlab-svgs/)`). + **size (optional)** Use one of the following sizes : 16,24,32,48,72 (this will be translated into a `s16` class) + **css_class (optional)** If you want to add additional css classes -**Example** +**Example** `= sprite_icon('issues', size: 72, css_class: 'icon-danger')` @@ -20,16 +22,34 @@ To use a sprite Icon in HAML or Rails we use a specific helper function : `<svg class="s72 icon-danger"><use xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="/assets/icons.svg#issues"></use></svg>` +### Usage in Vue + +We have a special Vue component for our sprite icons in `\vue_shared\components\icon.vue`. + +Sample usage : + +`<icon + name="retry" + :size="32" + css-classes="top" + />` + +**name** Name of the Icon in the SVG Sprite ([Overview is available here](http://gitlab-org.gitlab.io/gitlab-svgs/)`). + +**size (optional)** Number value for the size which is then mapped to a specific CSS class (Available Sizes: 8,12,16,18,24,32,48,72 are mapped to `sXX` css classes) + +**css-classes (optional)** Additional CSS Classes to add to the svg tag. + ### Usage in HTML/JS -Please use the following function inside JS to render an icon : +Please use the following function inside JS to render an icon : `gl.utils.spriteIcon(iconName)` ## Adding a new icon to the sprite All Icons and Illustrations are managed in the [gitlab-svgs](https://gitlab.com/gitlab-org/gitlab-svgs) repository which is added as a dev-dependency. -To upgrade to a new SVG Sprite version run `yarn upgrade https://gitlab.com/gitlab-org/gitlab-svgs` and then run `yarn run svg`. This task will copy the svg sprite and all illustrations in the correct folders. +To upgrade to a new SVG Sprite version run `yarn upgrade @gitlab-org/gitlab-svgs` and then run `yarn run svg`. This task will copy the svg sprite and all illustrations in the correct folders. The updated files should be tracked in Git as those are referenced. # SVG Illustrations diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 8f956681693..72cb557d054 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -71,12 +71,14 @@ Vue specific design patterns and practices. --- -## [Vue Resource](vue_resource.md) -Vue resource specific practices and gotchas. +## [Axios](axios.md) +Axios specific practices and gotchas. ## [Icons](icons.md) How we use SVG for our Icons. +## [Dropdowns](dropdowns.md) +How we use dropdowns. --- ## Style Guides diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index f88f0753687..6e9f18dd1c3 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -178,16 +178,13 @@ itself, please read this guide: [State Management][state-management] The Service is a class used only to communicate with the server. It does not store or manipulate any data. It is not aware of the store or the components. -We use [vue-resource][vue-resource-repo] to communicate with the server. -Refer to [vue resource](vue_resource.md) for more details. +We use [axios][axios] to communicate with the server. +Refer to [axios](axios.md) for more details. -Vue Resource should only be imported in the service file. +Axios instance should only be imported in the service file. ```javascript - import Vue from 'vue'; - import VueResource from 'vue-resource'; - - Vue.use(VueResource); + import axios from 'javascripts/lib/utils/axios_utils'; ``` ### End Result @@ -230,15 +227,14 @@ export default class Store { } // service.js -import Vue from 'vue'; -import VueResource from 'vue-resource'; -import 'vue_shared/vue_resource_interceptor'; - -Vue.use(VueResource); +import axios from 'javascripts/lib/utils/axios_utils' export default class Service { constructor(options) { - this.todos = Vue.resource(endpoint.todosEndpoint); + this.todos = axios.create({ + baseURL: endpoint.todosEndpoint + }); + } getTodos() { @@ -477,50 +473,8 @@ The main return value of a Vue component is the rendered output. In order to tes need to test the rendered output. [Vue][vue-test] guide's to unit test show us exactly that: ### Stubbing API responses -[Vue Resource Interceptors][vue-resource-interceptor] allow us to add a interceptor with -the response we need: - - ```javascript - // Mock the service to return data - const interceptor = (request, next) => { - next(request.respondWith(JSON.stringify([{ - title: 'This is a todo', - body: 'This is the text' - }]), { - status: 200, - })); - }; +Refer to [mock axios](axios.md#mock-axios-response-on-tests) - beforeEach(() => { - Vue.http.interceptors.push(interceptor); - }); - - afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, interceptor); - }); - - it('should do something', (done) => { - setTimeout(() => { - // Test received data - done(); - }, 0); - }); - ``` - -1. Headers interceptor -Refer to [this section](vue.md#headers) - -1. Use `$.mount()` to mount the component - -```javascript -// bad -new Component({ - el: document.createElement('div') -}); - -// good -new Component().$mount(); -``` ## Vuex To manage the state of an application you may use [Vuex][vuex-docs]. @@ -721,7 +675,6 @@ describe('component', () => { [component-system]: https://vuejs.org/v2/guide/#Composing-with-Components [state-management]: https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch [one-way-data-flow]: https://vuejs.org/v2/guide/components.html#One-Way-Data-Flow -[vue-resource-interceptor]: https://github.com/pagekit/vue-resource/blob/develop/docs/http.md#interceptors [vue-test]: https://vuejs.org/v2/guide/unit-testing.html [issue-boards-service]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/boards/services/board_service.js.es6 [flux]: https://facebook.github.io/flux @@ -729,3 +682,6 @@ describe('component', () => { [vuex-structure]: https://vuex.vuejs.org/en/structure.html [vuex-mutations]: https://vuex.vuejs.org/en/mutations.html [vuex-testing]: https://vuex.vuejs.org/en/testing.html +[axios]: https://github.com/axios/axios +[axios-interceptors]: https://github.com/axios/axios#interceptors + diff --git a/doc/development/fe_guide/vue_resource.md b/doc/development/fe_guide/vue_resource.md deleted file mode 100644 index c376c5c32bf..00000000000 --- a/doc/development/fe_guide/vue_resource.md +++ /dev/null @@ -1,72 +0,0 @@ -# Vue Resouce -In Vue applications we use [vue-resource][vue-resource-repo] to communicate with the server. - -## HTTP Status Codes - -### `.json()` -When making a request to the server, you will most likely need to access the body of the response. -Use `.json()` to convert. Because `.json()` returns a Promise the follwoing structure should be used: - - ```javascript - service.get('url') - .then(resp => resp.json()) - .then((data) => { - this.store.storeData(data); - }) - .catch(() => new Flash('Something went wrong')); - ``` - - -When using `Poll` (`app/assets/javascripts/lib/utils/poll.js`), the `successCallback` needs to handle `.json()` as a Promise: - ```javascript - successCallback: (response) => { - return response.json().then((data) => { - // handle the response - }); - } - ``` - -### 204 -Some endpoints - usually `delete` endpoints - return `204` as the success response. -When handling `204 - No Content` responses, we cannot use `.json()` since it tries to parse the non-existant body content. - -When handling `204` responses, do not use `.json`, otherwise the promise will throw an error and will enter the `catch` statement: - -```javascript - Vue.http.delete('path') - .then(() => { - // success! - }) - .catch(() => { - // handle error - }) -``` - -## Headers -Headers are being parsed into a plain object in an interceptor. -In Vue-resource 1.x `headers` object was changed into an `Headers` object. In order to not change all old code, an interceptor was added. - -If you need to write a unit test that takes the headers in consideration, you need to include an interceptor to parse the headers after your test interceptor. -You can see an example in `spec/javascripts/environments/environment_spec.js`: - ```javascript - import { headersInterceptor } from './helpers/vue_resource_helper'; - - beforeEach(() => { - Vue.http.interceptors.push(myInterceptor); - Vue.http.interceptors.push(headersInterceptor); - }); - - afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, myInterceptor); - Vue.http.interceptors = _.without(Vue.http.interceptors, headersInterceptor); - }); - ``` - -## CSRF token -We use a Vue Resource interceptor to manage the CSRF token. -`app/assets/javascripts/vue_shared/vue_resource_interceptor.js` holds all our common interceptors. -Note: You don't need to load `app/assets/javascripts/vue_shared/vue_resource_interceptor.js` -since it's already being loaded by `common_vue.js`. - - -[vue-resource-repo]: https://github.com/pagekit/vue-resource diff --git a/doc/development/file_storage.md b/doc/development/file_storage.md new file mode 100644 index 00000000000..cf00e24e11a --- /dev/null +++ b/doc/development/file_storage.md @@ -0,0 +1,49 @@ +# File Storage in GitLab + +We use the [CarrierWave] gem to handle file upload, store and retrieval. + +There are many places where file uploading is used, according to contexts: + +* System + - Instance Logo (logo visible in sign in/sign up pages) + - Header Logo (one displayed in the navigation bar) +* Group + - Group avatars +* User + - User avatars + - User snippet attachments +* Project + - Project avatars + - Issues/MR Markdown attachments + - Issues/MR Legacy Markdown attachments + - CI Build Artifacts + - LFS Objects + + +## Disk storage + +GitLab started saving everything on local disk. While directory location changed from previous versions, +they are still not 100% standardized. You can see them below: + +| Description | In DB? | Relative path | Uploader class | model_type | +| ------------------------------------- | ------ | ----------------------------------------------------------- | ---------------------- | ---------- | +| Instance logo | yes | uploads/-/system/appearance/logo/:id/:filename | `AttachmentUploader` | Appearance | +| Header logo | yes | uploads/-/system/appearance/header_logo/:id/:filename | `AttachmentUploader` | Appearance | +| Group avatars | yes | uploads/-/system/group/avatar/:id/:filename | `AvatarUploader` | Group | +| User avatars | yes | uploads/-/system/user/avatar/:id/:filename | `AvatarUploader` | User | +| User snippet attachments | yes | uploads/-/system/personal_snippet/:id/:random_hex/:filename | `PersonalFileUploader` | Snippet | +| Project avatars | yes | uploads/-/system/project/avatar/:id/:filename | `AvatarUploader` | Project | +| Issues/MR Markdown attachments | yes | uploads/:project_path_with_namespace/:random_hex/:filename | `FileUploader` | Project | +| Issues/MR Legacy Markdown attachments | no | uploads/-/system/note/attachment/:id/:filename | `AttachmentUploader` | Note | +| CI Artifacts (CE) | yes | shared/artifacts/:year_:month/:project_id/:id | `ArtifactUploader` | Ci::Build | +| LFS Objects (CE) | yes | shared/lfs-objects/:hex/:hex/:object_hash | `LfsObjectUploader` | LfsObject | + +CI Artifacts and LFS Objects behave differently in CE and EE. In CE they inherit the `GitlabUploader` +while in EE they inherit the `ObjectStoreUploader` and store files in and S3 API compatible object store. + +In the case of Issues/MR Markdown attachments, there is a different approach using the [Hashed Storage] layout, +instead of basing the path into a mutable variable `:project_path_with_namespace`, it's possible to use the +hash of the project ID instead, if project migrates to the new approach (introduced in 10.2). + +[CarrierWave]: https://github.com/carrierwaveuploader/carrierwave +[Hashed Storage]: ../administration/repository_storage_types.md diff --git a/doc/development/github_importer.md b/doc/development/github_importer.md new file mode 100644 index 00000000000..0d558583bb8 --- /dev/null +++ b/doc/development/github_importer.md @@ -0,0 +1,209 @@ +# Working with the GitHub importer + +In GitLab 10.2 a new version of the GitHub importer was introduced. This new +importer performs its work in parallel using Sidekiq, greatly reducing the time +necessary to import GitHub projects into a GitLab instance. + +The GitHub importer offers two different types of importers: a sequential +importer and a parallel importer. The Rake task `import:github` uses the +sequential importer, while everything else uses the parallel importer. The +difference between these two importers is quite simple: the sequential importer +does all work in a single thread, making it more useful for debugging purposes +or Rake tasks. The parallel importer on the other hand uses Sidekiq. + +## Requirements + +* GitLab CE 10.2.0 or newer. +* Sidekiq workers that process the `github_importer` and + `github_importer_advance_stage` queues (this is enabled by default). +* Octokit (used for interacting with the GitHub API) + +## Code structure + +The importer's codebase is broken up into the following directories: + +* `lib/gitlab/github_import`: this directory contains most of the code such as + the classes used for importing resources. +* `app/workers/gitlab/github_import`: this directory contains the Sidekiq + workers. +* `app/workers/concerns/gitlab/github_import`: this directory contains a few + modules reused by the various Sidekiq workers. + +## Architecture overview + +When a GitHub project is imported we schedule and execute a job for the +`RepositoryImportworker` worker as all other importers. However, unlike other +importers we don't immediately perform the work necessary. Instead work is +divided into separate stages, with each stage consisting out of a set of Sidekiq +jobs that are executed. Between every stage a job is scheduled that periodically +checks if all work of the current stage is completed, advancing the import +process to the next stage when this is the case. The worker handling this is +called `Gitlab::GithubImport::AdvanceStageWorker`. + +## Stages + +### 1. RepositoryImportWorker + +This worker will kick off the import process by simply scheduling a job for the +next worker. + +### 2. Stage::ImportRepositoryWorker + +This worker will import the repository and wiki, scheduling the next stage when +done. + +### 3. Stage::ImportBaseDataWorker + +This worker will import base data such as labels, milestones, and releases. This +work is done in a single thread since it can be performed fast enough that we +don't need to perform this work in parallel. + +### 4. Stage::ImportPullRequestsWorker + +This worker will import all pull requests. For every pull request a job for the +`Gitlab::GithubImport::ImportPullRequestWorker` worker is scheduled. + +### 5. Stage::ImportIssuesAndDiffNotesWorker + +This worker will import all issues and pull request comments. For every issue we +schedule a job for the `Gitlab::GithubImport::ImportIssueWorker` worker. For +pull request comments we instead schedule jobs for the +`Gitlab::GithubImport::DiffNoteImporter` worker. + +This worker processes both issues and diff notes in parallel so we don't need to +schedule a separate stage and wait for the previous one to complete. + +Issues are imported separately from pull requests because only the "issues" API +includes labels for both issue and pull requests. Importing issues and setting +label links in the same worker removes the need for performing a separate crawl +through the API data, reducing the number of API calls necessary to import a +project. + +### 6. Stage::ImportNotesWorker + +This worker imports regular comments for both issues and pull requests. For +every comment we schedule a job for the +`Gitlab::GithubImport::ImportNoteWorker` worker. + +Regular comments have to be imported at the end since the GitHub API used +returns comments for both issues and pull requests. This means we have to wait +for all issues and pull requests to be imported before we can import regular +comments. + +### 7. Stage::FinishImportWorker + +This worker will wrap up the import process by performing some housekeeping +(such as flushing any caches) and by marking the import as completed. + +## Advancing stages + +Advancing stages is done in one of two ways: + +1. Scheduling the worker for the next stage directly. +2. Scheduling a job for `Gitlab::GithubImport::AdvanceStageWorker` which will + advance the stage when all work of the current stage has been completed. + +The first approach should only be used by workers that perform all their work in +a single thread, while `AdvanceStageWorker` should be used for everything else. + +The way `AdvanceStageWorker` works is fairly simple. When scheduling a job it +will be given a project ID, a list of Redis keys, and the name of the next +stage. The Redis keys (produced by `Gitlab::JobWaiter`) are used to check if the +currently running stage has been completed or not. If the stage has not yet been +completed `AdvanceStageWorker` will reschedule itself. Once a stage finishes +`AdvanceStageworker` will refresh the import JID (more on this below) and +schedule the worker of the next stage. + +To reduce the number of `AdvanceStageWorker` jobs scheduled this worker will +briefly wait for jobs to complete before deciding what the next action should +be. For small projects this may slow down the import process a bit, but it will +also reduce pressure on the system as a whole. + +## Refreshing import JIDs + +GitLab includes a worker called `StuckImportJobsWorker` that will periodically +run and mark project imports as failed if they have been running for more than +15 hours. For GitHub projects this poses a bit of a problem: importing large +projects could take several hours depending on how often we hit the GitHub rate +limit (more on this below), but we don't want `StuckImportJobsWorker` to mark +our import as failed because of this. + +To prevent this from happening we periodically refresh the expiration time of +the import process. This works by storing the JID of the import job in the +database, then refreshing this JID's TTL at various stages throughout the import +process. This is done by calling `Project#refresh_import_jid_expiration`. By +refreshing this TTL we can ensure our import does not get marked as failed so +long we're still performing work. + +## GitHub rate limit + +GitHub has a rate limit of 5 000 API calls per hour. The number of requests +necessary to import a project is largely dominated by the number of unique users +involved in a project (e.g. issue authors). Other data such as issue pages +and comments typically only requires a few dozen requests to import. This is +because we need the Email address of users in order to map them to GitLab users. + +We handle this by doing the following: + +1. Once we hit the rate limit all jobs will automatically reschedule themselves + in such a way that they are not executed until the rate limit has been reset. +2. We cache the mapping of GitHub users to GitLab users in Redis. + +More information on user caching can be found below. + +## Caching user lookups + +When mapping GitHub users to GitLab users we need to (in the worst case) +perform: + +1. One API call to get the user's Email address. +2. Two database queries to see if a corresponding GitLab user exists. One query + will try to find the user based on the GitHub user ID, while the second query + is used to find the user using their GitHub Email address. + +Because this process is quite expensive we cache the result of these lookups in +Redis. For every user looked up we store three keys: + +1. A Redis key mapping GitHub usernames to their Email addresses. +2. A Redis key mapping a GitHub Email addresses to a GitLab user ID. +3. A Redis key mapping a GitHub user ID to GitLab user ID. + +There are two types of lookups we cache: + +1. A positive lookup, meaning we found a GitLab user ID. +2. A negative lookup, meaning we didn't find a GitLab user ID. Caching this + prevents us from performing the same work for users that we know don't exist + in our GitLab database. + +The expiration time of these keys is 24 hours. When retrieving the cache of a +positive lookups we refresh the TTL automatically. The TTL of false lookups is +never refreshed. + +Because of this caching layer it's possible newly registered GitLab accounts +won't be linked to their corresponding GitHub accounts. This however will sort +itself out once the cached keys expire. + +The user cache lookup is shared across projects. This means that the more +projects get imported the fewer GitHub API calls will be needed. + +The code for this resides in: + +* `lib/gitlab/github_import/user_finder.rb` +* `lib/gitlab/github_import/caching.rb` + +## Mapping labels and milestones + +To reduce pressure on the database we do not query it when setting labels and +milestones on issues and merge requests. Instead we cache this data when we +import labels and milestones, then we reuse this cache when assigning them to +issues/merge requests. Similar to the user lookups these cache keys are expired +automatically after 24 hours of not being used. + +Unlike the user lookup caches these label and milestone caches are scoped to the +project that is being imported. + +The code for this resides in: + +* `lib/gitlab/github_import/label_finder.rb` +* `lib/gitlab/github_import/milestone_finder.rb` +* `lib/gitlab/github_import/caching.rb` diff --git a/doc/development/i18n/externalization.md b/doc/development/i18n/externalization.md index 7c38260406d..4b65a0f4a35 100644 --- a/doc/development/i18n/externalization.md +++ b/doc/development/i18n/externalization.md @@ -110,7 +110,7 @@ You can mark that content for translation with: In JavaScript we added the `__()` (double underscore parenthesis) function for translations. -### Updating the PO files with the new content +## Updating the PO files with the new content Now that the new content is marked for translation, we need to update the PO files with the following command: @@ -119,23 +119,20 @@ files with the following command: bundle exec rake gettext:find ``` -This command will update the `locale/**/gitlab.edit.po` file with the -new content that the parser has found. +This command will update the `locale/gitlab.pot` file with the newly externalized +strings and remove any strings that aren't used anymore. You should check this +file in. Once the changes are on master, they will be picked up by +[Crowdin](http://translate.gitlab.com) and be presented for translation. -New translations will be added with their default content and will be marked -fuzzy. To use the translation, look for the `#, fuzzy` mention in `gitlab.edit.po` -and remove it. +The command also updates the translation files for each language: `locale/*/gitlab.po` +These changes can be discarded, the languange files will be updated by Crowdin +automatically. -We need to make sure we remove the `fuzzy` translations before generating the -`locale/**/gitlab.po` file. When they aren't removed, the resulting `.po` will -be treated as a binary file which could overwrite translations that were merged -before the new translations. +Discard all of them at once like this: -When we are just preparing a page to be translated, but not actually adding any -translations. There's no need to generate `.po` files. - -Translations that aren't used in the source code anymore will be marked with -`~#`; these can be removed to keep our translation files clutter-free. +```sh +git checkout locale/*/gitlab.po +``` ### Validating PO files diff --git a/doc/development/licensing.md b/doc/development/licensing.md index 902b1c74a42..274923c2d43 100644 --- a/doc/development/licensing.md +++ b/doc/development/licensing.md @@ -4,11 +4,11 @@ GitLab CE is licensed under the terms of the MIT License. GitLab EE is licensed ## Automated Testing -In order to comply with the terms the libraries we use are licensed under, we have to make sure to check new gems for compatible licenses whenever they're added. To automate this process, we use the [license_finder][license_finder] gem by Pivotal. It runs every time a new commit is pushed and verifies that all gems in the bundle use a license that doesn't conflict with the licensing of either GitLab Community Edition or GitLab Enterprise Edition. +In order to comply with the terms the libraries we use are licensed under, we have to make sure to check new gems for compatible licenses whenever they're added. To automate this process, we use the [license_finder][license_finder] gem by Pivotal. It runs every time a new commit is pushed and verifies that all gems and node modules in the bundle use a license that doesn't conflict with the licensing of either GitLab Community Edition or GitLab Enterprise Edition. -There are some limitations with the automated testing, however. CSS and JavaScript libraries, as well as any Ruby libraries not included by way of Bundler, must be verified manually and independently. Take care whenever one such library is used, as automated tests won't catch problematic licenses from them. +There are some limitations with the automated testing, however. CSS, JavaScript, or Ruby libraries which are not included by way of Bundler, NPM, or Yarn (for instance those manually copied into our source tree in the `vendor` directory), must be verified manually and independently. Take care whenever one such library is used, as automated tests won't catch problematic licenses from them. -Some gems may not include their license information in their `gemspec` file. These won't be detected by License Finder, and will have to be verified manually. +Some gems may not include their license information in their `gemspec` file, and some node modules may not include their license information in their `package.json` file. These won't be detected by License Finder, and will have to be verified manually. ### License Finder commands diff --git a/doc/development/limit_ee_conflicts.md b/doc/development/limit_ee_conflicts.md index 899be9eae4b..ba82babb38a 100644 --- a/doc/development/limit_ee_conflicts.md +++ b/doc/development/limit_ee_conflicts.md @@ -336,6 +336,12 @@ Blocks of code that are EE-specific should be moved to partials as much as possible to avoid conflicts with big chunks of HAML code that that are not fun to resolve when you add the indentation in the equation. +### Assets + +#### gitlab-svgs + +Conflicts in `app/assets/images/icons.json` or `app/assets/images/icons.svg` can be resolved simply by regenerating those assets with [`yarn run svg`](https://gitlab.com/gitlab-org/gitlab-svgs). + --- [Return to Development documentation](README.md) diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 9b8ab5da74e..a235dd74909 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -198,7 +198,43 @@ end Keep in mind that this operation can easily take 10-15 minutes to complete on larger installations (e.g. GitLab.com). As a result you should only add default -values if absolutely necessary. +values if absolutely necessary. There is a RuboCop cop that will fail if this +method is used on some tables that are very large on GitLab.com, which would +cause other issues. + +## Updating an existing column + +To update an existing column to a particular value, you can use +`update_column_in_batches` (`add_column_with_default` uses this internally to +fill in the default value). This will split the updates into batches, so we +don't update too many rows at in a single statement. + +This updates the column `foo` in the `projects` table to 10, where `some_column` +is `'hello'`: + +```ruby +update_column_in_batches(:projects, :foo, 10) do |table, query| + query.where(table[:some_column].eq('hello')) +end +``` + +To perform a computed update, the value can be wrapped in `Arel.sql`, so Arel +treats it as an SQL literal. The below example is the same as the one above, but +the value is set to the product of the `bar` and `baz` columns: + +```ruby +update_value = Arel.sql('bar * baz') + +update_column_in_batches(:projects, :foo, update_value) do |table, query| + query.where(table[:some_column].eq('hello')) +end +``` + +Like `add_column_with_default`, there is a RuboCop cop to detect usage of this +on large tables. In the case of `update_column_in_batches`, it may be acceptable +to run on a large table, as long as it is only updating a small subset of the +rows in the table, but do not ignore that without validating on the GitLab.com +staging environment - or asking someone else to do so for you - beforehand. ## Integer column type diff --git a/doc/development/query_recorder.md b/doc/development/query_recorder.md index e0127aaed4c..12e90101139 100644 --- a/doc/development/query_recorder.md +++ b/doc/development/query_recorder.md @@ -22,6 +22,52 @@ As an example you might create 5 issues in between counts, which would cause the > **Note:** In some cases the query count might change slightly between runs for unrelated reasons. In this case you might need to test `exceed_query_limit(control_count + acceptable_change)`, but this should be avoided if possible. +## Finding the source of the query + +It may be useful to identify the source of the queries by looking at the call backtrace. +To enable this, run the specs with the `QUERY_RECORDER_DEBUG` environment variable set. For example: + +``` +QUERY_RECORDER_DEBUG=1 bundle exec rspec spec/requests/api/projects_spec.rb +``` + +This will log calls to QueryRecorder into the `test.log`. For example: + +``` +QueryRecorder SQL: SELECT COUNT(*) FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."project_id" = $1 AND ("issues"."state" IN ('opened')) AND "issues"."confidential" = $2 + --> /home/user/gitlab/gdk/gitlab/spec/support/query_recorder.rb:19:in `callback' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/fanout.rb:127:in `finish' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/fanout.rb:46:in `block in finish' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/fanout.rb:46:in `each' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/fanout.rb:46:in `finish' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/instrumenter.rb:36:in `finish' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/instrumenter.rb:25:in `instrument' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract_adapter.rb:478:in `log' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:601:in `exec_cache' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:585:in `execute_and_clear' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql/database_statements.rb:160:in `exec_query' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/database_statements.rb:356:in `select' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/database_statements.rb:32:in `select_all' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/query_cache.rb:68:in `block in select_all' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/query_cache.rb:83:in `cache_sql' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/query_cache.rb:68:in `select_all' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/relation/calculations.rb:270:in `execute_simple_calculation' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/relation/calculations.rb:227:in `perform_calculation' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/relation/calculations.rb:133:in `calculate' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/relation/calculations.rb:48:in `count' + --> /home/user/gitlab/gdk/gitlab/app/services/base_count_service.rb:20:in `uncached_count' + --> /home/user/gitlab/gdk/gitlab/app/services/base_count_service.rb:12:in `block in count' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:299:in `block in fetch' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:585:in `block in save_block_result_to_cache' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:547:in `block in instrument' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications.rb:166:in `instrument' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:547:in `instrument' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:584:in `save_block_result_to_cache' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:299:in `fetch' + --> /home/user/gitlab/gdk/gitlab/app/services/base_count_service.rb:12:in `count' + --> /home/user/gitlab/gdk/gitlab/app/models/project.rb:1296:in `open_issues_count' +``` + ## See also - [Bullet](profiling.md#Bullet) For finding `N+1` query problems diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index bfd80aab6a4..ceff57276d2 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -122,6 +122,15 @@ they can be easily inspected. bundle exec rake services:doc ``` +## Updating Emoji Aliases + +To update the Emoji aliases file (used for Emoji autocomplete) you must run the +following: + +``` +bundle exec rake gemojione:aliases +``` + ## Updating Emoji Digests To update the Emoji digests file (used for Emoji autocomplete) you must run the @@ -131,6 +140,7 @@ following: bundle exec rake gemojione:digests ``` + This will update the file `fixtures/emojis/digests.json` based on the currently available Emoji. @@ -153,7 +163,7 @@ Starting a project from a template needs this project to be exported. On a up to date master branch with run: ``` -gdk run db +gdk run # In a new terminal window bundle exec rake gitlab:update_project_templates git checkout -b update-project-templates diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index 1e9fdbc65e2..085fb8f902c 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -3,6 +3,12 @@ This document outlines various guidelines that should be followed when adding or modifying Sidekiq workers. +## ApplicationWorker + +All workers should include `ApplicationWorker` instead of `Sidekiq::Worker`, +which adds some convenience methods and automatically sets the queue based on +the worker's name. + ## Default Queue Use of the "default" queue is not allowed. Every worker should use a queue that @@ -13,19 +19,10 @@ A list of all available queues can be found in `config/sidekiq_queues.yml`. ## Dedicated Queues -Most workers should use their own queue. To ease this process a worker can -include the `DedicatedSidekiqQueue` concern as follows: - -```ruby -class ProcessSomethingWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue -end -``` - -This will set the queue name based on the class' name, minus the `Worker` -suffix. In the above example this would lead to the queue being -`process_something`. +Most workers should use their own queue, which is automatically set based on the +worker class name. For a worker named `ProcessSomethingWorker`, the queue name +would be `process_something`. If you're not sure what a worker's queue name is, +you can find it using `SomeWorker.queue`. In some cases multiple workers do use the same queue. For example, the various workers for updating CI pipelines all use the `pipeline` queue. Adding workers @@ -39,7 +36,7 @@ tests should be placed in `spec/workers`. ## Removing or renaming queues -Try to avoid renaming or removing queues in minor and patch releases. -During online update instance can have pending jobs and removing the queue can -lead to those jobs being stuck forever. If you can't write migration for those -Sidekiq jobs, please consider doing rename or remove queue in major release only.
\ No newline at end of file +Try to avoid renaming or removing queues in minor and patch releases. +During online update instance can have pending jobs and removing the queue can +lead to those jobs being stuck forever. If you can't write migration for those +Sidekiq jobs, please consider doing rename or remove queue in major release only. diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index c4830322fa8..05e0a64af18 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -37,7 +37,7 @@ when using the migration helper method `Gitlab::Database::MigrationHelpers#add_column_with_default`. This method works similar to `add_column` except it updates existing rows in batches without blocking access to the table being modified. See ["Adding Columns With Default -Values"](migration_style_guide.html#adding-columns-with-default-values) for more +Values"](migration_style_guide.md#adding-columns-with-default-values) for more information on how to use this method. ## Dropping Columns diff --git a/doc/development/writing_documentation.md b/doc/development/writing_documentation.md index 68ba3dd2da3..b6def7ef541 100644 --- a/doc/development/writing_documentation.md +++ b/doc/development/writing_documentation.md @@ -152,12 +152,23 @@ CE and EE. ## Previewing the changes live If you want to preview the doc changes of your merge request live, you can use -the manual `review-docs-deploy` job in your merge request. +the manual `review-docs-deploy` job in your merge request. You will need at +least Master permissions to be able to run it and is currently enabled for the +following projects: + +- https://gitlab.com/gitlab-org/gitlab-ce +- https://gitlab.com/gitlab-org/gitlab-ee + +NOTE: **Note:** +You will need to push a branch to those repositories, it doesn't work for forks. TIP: **Tip:** If your branch contains only documentation changes, you can use [special branch names](#testing) to avoid long running pipelines. +In the mini pipeline graph, you should see an `>>` icon. Clicking on it will +reveal the `review-docs-deploy` job. Hit the play button for the job to start. +  This job will: |