Skip to content
Snippets Groups Projects

Add variables API

Merged Add variables API
ci/api-variables into master
Merged Tomasz Maczukin requested to merge ci/api-variables into master

References #4264 (closed)

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Tomasz Maczukin Added 8 commits:

    Added 8 commits:

    • 73382033...2c1f8e2d - 6 commits from branch master
    • ea4777ff - Add features for list and show details of variables in API
    • a692ce1c - Add update feature for variables API
  • Tomasz Maczukin Added 2 commits:

    Added 2 commits:

    • 0d014feb - Add delete feature to variables API
    • c5177dd5 - Add missing 'not_found' checks in variables API
  • Tomasz Maczukin Added 1 commit:

    Added 1 commit:

    • 937567b7 - Add create feature to variables API
  • Tomasz Maczukin Added 1 commit:

    Added 1 commit:

    • 16bd4df0 - Fix a typo in method description
  • Tomasz Maczukin
    Tomasz Maczukin @tmaczukin ·
    Author Maintainer

    @ayufan Ready for review :smile:

  • Tomasz Maczukin mentioned in merge request !2286 (merged)

    mentioned in merge request !2286 (merged)

  • Kamil Trzcińśki
    Kamil Trzcińśki @ayufan ·
    Maintainer

    We should add validation to limit the key name to: [A-Za-z0-9_].

  • Kamil Trzcińśki
  • Tomasz Maczukin Added 2 commits:

    Added 2 commits:

    • b60c1462 - Change :variable_id to :key as resource ID in API
    • b6044590 - Update ./doc/api
  • Tomasz Maczukin Added 1 commit:

    Added 1 commit:

  • Kamil Trzcińśki
  • Kamil Trzcińśki
    Kamil Trzcińśki @ayufan started a thread on the diff
    • lib/api/variables.rb 0 → 100644
    37 # Create a new variable in project
    38 #
    39 # Parameters:
    40 # id (required) - The ID of a project
    41 # key (required) - The key of variable
    42 # value (required) - The value of variable
    43 # Example Request:
    44 # POST /projects/:id/variables
    45 post ':id/variables' do
    46 required_attributes! [:key, :value]
    47
    48 variable = user_project.variables.create(key: params[:key], value: params[:value])
    49 return render_validation_error!(variable) unless variable.valid?
    50 variable.save!
    51
    52 present variable, with: Entities::Variable
  • Kamil Trzcińśki
  • Kamil Trzcińśki
  • Kamil Trzcińśki
    Kamil Trzcińśki @ayufan ·
    Maintainer

    @DouweM @rspeicher Can you review?

  • Tomasz Maczukin Added 1 commit:

    Added 1 commit:

    • df548285 - Add some fixes after review
  • Tomasz Maczukin Title changed from [WIP] Add variables API to Add variables API

    Title changed from [WIP] Add variables API to Add variables API

  • Douwe Maan
  • Douwe Maan
    Douwe Maan @DouweM started a thread on the diff
    • app/models/ci/variable.rb
    17 18
    18 19 belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id
    19 20
    20 validates_presence_of :key
    21 21 validates_uniqueness_of :key, scope: :gl_project_id
    22 validates :key,
    23 presence: true,
    24 length: { within: 0..255 },
    25 format: { with: /\A[a-zA-Z0-9_]+\z/,
    • Douwe Maan
      Douwe Maan @DouweM ·
      Maintainer

      Do we want lowercase chars?

    • Robert Speicher
      Robert Speicher @rspeicher ·
      Owner

      :thumbsup: All the examples are uppercase-only.

    • Tomasz Maczukin
      Tomasz Maczukin @tmaczukin ·
      Author Maintainer

      What stands against lowercase chars in variables names? Technically this set of characters is safe to use as ID in URL and as variable name in shell.

      And as for examples - we can (and even we should) add the information about allowed characters in variables API documentation. Example is only an example. The documentation and the validation messages says to user what can they do.

    • Kamil Trzcińśki
      Kamil Trzcińśki @ayufan ·
      Maintainer

      @DouweM Yes, we do want. It is valid to have lowercase environment variables.

  • Robert Speicher
  • Robert Speicher
  • Robert Speicher
  • Robert Speicher
  • Robert Speicher
  • Tomasz Maczukin Added 1 commit:

    Added 1 commit:

  • Tomasz Maczukin Added 2 commits:

    Added 2 commits:

    • d3380876 - Add 'Build' prefix to Variables entry name in API docs index
    • 3b7f3428 - Modify :ci_variable factory
  • Tomasz Maczukin Added 1 commit:

    Added 1 commit:

    • 2b81248a - Modify builds API documentation style [ci skip]
  • Tomasz Maczukin Added 448 commits:

    Added 448 commits:

    • 2b81248a...0e344aa2 - 446 commits from branch master
    • 6c4e8536 - Merge branch 'master' into ci/api-variables
    • bc1fab45 - Update CHANGELOG [ci skip]
  • Tomasz Maczukin
    Tomasz Maczukin @tmaczukin ·
    Author Maintainer

    @rspeicher @DouweM: I've done some modifications according to the suggestions. I also added CHANGELOG entry.

    There is still question about key validation. What to do with the regexp?

  • Robert Speicher
    Robert Speicher @rspeicher ·
    Owner

    I think allowing mixed case is fine.

    Pinging @axil - Check out your new API doc standards in action!

  • Achilleas Pipinellis
    Achilleas Pipinellis @axil ·
    Maintainer

    Ooh, neat! Thanks @tmaczukin :)

    Some general advice (which I forgot to document, so it's my fault):

    Do not add headings for Parameters, Example of request and Example of response, add only for the main calls. Always imagine that the headings end up being referenced in doc.gitlab.com. How would you know what is referenced if your URL is api/build_variables.md#parameters ? If you have multiple times the same heading you would not know where to go.

    Other than that I'll leave some inline comments.

  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
  • Achilleas Pipinellis
    Achilleas Pipinellis @axil started a thread on the diff
    • doc/api/build_variables.md 0 → 100644
    135 DELETE /projects/:id/variables/:key
    136 ```
    137
    138 ### Parameters
    139
    140 | Attribute | Type | required | Description |
    141 |-----------|---------|----------|-------------------------|
    142 | id | integer | yes | The ID of a project |
    143 | key | string | yes | The `key` of variable |
    144
    145 ### Example of request
    146
    147 ```
    148 curl -X DELETE -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/VARIABLE_1"
    149 ```
    150
  • Tomasz Maczukin Added 1 commit:

    Added 1 commit:

    • c230caa2 - Add some cosmetic changes to variables API documentation [ci skip]
  • Tomasz Maczukin Added 2 commits:

    Added 2 commits:

    • bf21e796 - Update CHANGELOG [ci skip]
    • d5359dd5 - Add some cosmetic changes to variables API documentation [ci skip]
  • Tomasz Maczukin Added 2 commits:

    Added 2 commits:

    • f11ba743 - Add some cosmetic changes to variables API documentation [ci skip]
    • 2344e006 - Update CHANGELOG [ci skip]
  • Tomasz Maczukin Target branch changed from master to 8-4-stable

    Target branch changed from master to 8-4-stable

  • Tomasz Maczukin Added 449 commits:

    Added 449 commits:

    • 2344e006...d139276a - 448 commits from branch 8-4-stable
    • b6f91e4b - Merge branch '8-4-stable' into ci/api-variables
  • Tomasz Maczukin Added 2 commits:

    Added 2 commits:

    • 220c3a44 - Merge branch '8-4-stable' into ci/api-variables
    • eee16ca9 - Update CHANGELOG [ci skip]
  • Achilleas Pipinellis
    Achilleas Pipinellis @axil ·
    Maintainer

    :thumbsup: from me, ready to merge.

  • Douwe Maan Target branch changed from 8-4-stable to master

    Target branch changed from 8-4-stable to master

  • Douwe Maan Added 40 commits:

    Added 40 commits:

  • Douwe Maan
    Douwe Maan @DouweM ·
    Maintainer

    @tmaczukin I changed the target branch back to master. All MRs should go into master, stable branches are updated when appropriate by merging master into them, or by cherry-picking specific commits.

  • Douwe Maan Added 1 commit:

    Added 1 commit:

  • Douwe Maan Enabled an automatic merge when the build for 4435a3a8 succeeds

    Enabled an automatic merge when the build for 4435a3a8 succeeds

  • Douwe Maan mentioned in commit f981da44

    mentioned in commit f981da44

  • Douwe Maan Status changed to merged

    Status changed to merged

  • Robert Speicher
    Robert Speicher @rspeicher ·
    Owner

    Picked into 8-4-stable.

  • Douwe Maan mentioned in commit 2a89073c

    mentioned in commit 2a89073c

  • Achilleas Pipinellis mentioned in issue #10852 (closed)

    mentioned in issue #10852 (closed)

  • Achilleas Pipinellis mentioned in issue #10421 (moved)

    mentioned in issue #10421 (moved)

  • Tomasz Maczukin mentioned in merge request gitlab-com/www-gitlab-com!1722 (merged)

    mentioned in merge request gitlab-com/www-gitlab-com!1722 (merged)

  • Douwe Maan Mentioned in commit maxraab/gitlab-ce@2a89073c

    Mentioned in commit maxraab/gitlab-ce@2a89073c

    • Please register or sign in to reply