Integrate obs factory #5140

DavidKang · 2018-06-14T08:49:11Z

We made some adjustments to integrate obs_factory engine into OBS.
We still need to test the correct workflows and fix rubocop complains.

Ana06

What a huge PR! 😨

src/api/app/models/obs_factory/distribution.rb

src/api/app/helpers/webui/obs_factory/application_helper.rb

src/api/app/controllers/webui/obs_factory/staging_projects_controller.rb

src/api/app/models/obs_factory/distribution.rb

hennevogel · 2018-06-14T09:47:31Z

@mdeniz go change some diapers or something! ☝

src/api/config/routes.rb

Ana06 · 2018-06-18T15:48:48Z

src/api/app/controllers/webui/obs_factory/staging_projects_controller.rb

+          unless file.to_s.nil?
+            @ignored_requests = YAML.load(file.to_s)
+          end
+          if !@ignored_requests.nil? and @ignored_requests


This code needs to be rewritten completely in the future anyway as this should move into an obs attribute. Polishing this old functionality to perfection is beyong logic.

The scarier part is the need for unless file.to_s.nil?...to_s.nil?

It's actually no longer needed. Your code was using PackageFile.new - which has a rather strange API as it tries to mimik ActiveRecord::Model.

But @DavidKang changed it to use the file method - and that one simply returns a string or throws an exception.

jberry-suse a day ago
The scarier part is the need for unless file.to_s.nil?...to_s.nil?

I was also suprised... We renamed it now

Ana06

@DavidKang please add in the commit what code was just move to avoid adding comments in that code. 😄

bgeuken

Nice work. I liked seeing you already cleaning up the CSS and such ❤️

There is just one thing. It seems that the first 3 commits belong together. The second one adds some name spacing which seems to be required for the first commit to work.
The third commit drops a controller which got introduced in the first one.
I would also consider squashing the 4th commit

DavidKang · 2018-06-19T15:17:19Z

@bgeuken, I prefer to leave the first commit separated because is only moving obs factory Engine into OBS and it will be easier to review the others commits.

coolo

Be careful on deployment as we need to remove the engine at the same time. I'll gladly help with first-deployment debugging :)

DavidKang · 2018-06-20T08:18:55Z

@coolo, thanks 😸

bgeuken · 2018-06-20T08:26:53Z

Probably obvious to say, but I would test this thoroughly before deploying. Be it on https://build-test.opensuse.org/ or on a local dev instance.

coolo · 2018-06-20T08:30:11Z

I can setup a Factory clone on build-test

bgeuken · 2018-06-20T09:55:52Z

I am building packages from
@DavidKang's branch: https://build.opensuse.org/package/show/home:bgeuken:branches:OBS:Server:Unstable/obs-server

coolo · 2018-06-21T09:59:55Z

I deployed this and setup a minimal https://build-test.opensuse.org/project/staging_projects/openSUSE:Factory

But if there is no ignored_requests file, it will crash - due to the backend refactoring. So please catch this exception - and we're ready for merge IMO

coolo · 2018-06-21T10:00:19Z

I, [2018-06-21T11:54:11.465161 #10305]  INFO -- : [57aef12f-29ac-4402-9eaa-da3202c19afc] [10305:177.22] method=GET path=/project/staging_projects/openSUSE:Factory format=html controller=We
bui::ObsFactory::StagingProjectsController action=index status=500 error='ActiveXML::Transport::NotFoundError: <status code="404">
  <summary>package 'dashboard' does not exist</summary>
  <details>404 package 'dashboard' does not exist</details>
</status>

DavidKang · 2018-06-21T12:16:43Z

@coolo, @bgeuken, @eduardoj and @Ana06, could re-review it again. I made some changes in the commit b63b3ef

We wanted to add the OBS Factory engine in this project to be part of the code of OBS. This commit only move OBS Factory Engine files into OBS.

Some of the name where clashing with some global ones. Now we prepend the name of the module and the global accessor. Also some unused code, controller and old routes were removed.

bgeuken · 2018-06-21T13:21:01Z

@DavidKang The commit looks good to me

coolo · 2018-06-21T14:15:33Z

@Ana06?

jberry-suse · 2018-06-21T17:45:40Z

src/api/app/controllers/webui/obs_factory/staging_projects_controller.rb

+              @ignored_requests = YAML.load(file)
+            end
+          else
+            flash[:notice] = "package dashboard doesn't exist for #{staging_project}"


A dashboard package is not a requirement and as such this will be annoying for projects without one.

Okey, I will remove it

We replaced the use of ActiveXML and PackageFile in favour of Backend::Api.

We updated rubocop_todo file to exclude all the offenses related with obs_factory.

stale review

DavidKang added Frontend Things related to the OBS RoR app DO NOT MERGE ⚠️ Explain yourself if you add/remove this label to a PR labels Jun 14, 2018

DavidKang requested a review from coolo June 14, 2018 08:49

Ana06 reviewed Jun 14, 2018

View reviewed changes

mdeniz reviewed Jun 14, 2018

View reviewed changes

src/api/app/models/obs_factory/distribution.rb Outdated Show resolved Hide resolved

mdeniz reviewed Jun 14, 2018

View reviewed changes

src/api/app/helpers/webui/obs_factory/application_helper.rb Outdated Show resolved Hide resolved

mdeniz reviewed Jun 14, 2018

View reviewed changes

src/api/app/controllers/webui/obs_factory/staging_projects_controller.rb Outdated Show resolved Hide resolved

mdeniz reviewed Jun 14, 2018

View reviewed changes

src/api/app/models/obs_factory/distribution.rb Outdated Show resolved Hide resolved

DavidKang force-pushed the feature/obs-factory-without-test branch 2 times, most recently from 57857e2 to e648327 Compare June 14, 2018 09:30

coolo reviewed Jun 14, 2018

View reviewed changes

src/api/config/routes.rb Outdated Show resolved Hide resolved

src/api/config/routes.rb Outdated Show resolved Hide resolved

DavidKang force-pushed the feature/obs-factory-without-test branch 3 times, most recently from 6b52e0b to eb35b94 Compare June 14, 2018 11:56

coolo mentioned this pull request Jun 17, 2018

Use attribute to store staging config openSUSE/openSUSE-release-tools#1573

Merged

DavidKang force-pushed the feature/obs-factory-without-test branch from eb35b94 to 5917f49 Compare June 18, 2018 11:36

DavidKang changed the title ~~WIP Integrate obs factory~~ Integrate obs factory Jun 18, 2018

DavidKang removed the DO NOT MERGE ⚠️ Explain yourself if you add/remove this label to a PR label Jun 18, 2018

DavidKang force-pushed the feature/obs-factory-without-test branch 5 times, most recently from b236bb1 to 03f04dc Compare June 18, 2018 14:40

Ana06 previously requested changes Jun 18, 2018

View reviewed changes

Ana06 reviewed Jun 18, 2018

View reviewed changes

bgeuken suggested changes Jun 19, 2018

View reviewed changes

DavidKang force-pushed the feature/obs-factory-without-test branch from 03f04dc to f6a4a08 Compare June 19, 2018 08:48

eduardoj approved these changes Jun 19, 2018

View reviewed changes

coolo approved these changes Jun 20, 2018

View reviewed changes

coolo mentioned this pull request Jun 20, 2018

RFC:Add external api to OBS openSUSE/obs_factory#48

Closed

bgeuken approved these changes Jun 20, 2018

View reviewed changes

DavidKang force-pushed the feature/obs-factory-without-test branch from f6a4a08 to 6c3257f Compare June 21, 2018 11:21

Moises Deniz Aleman and others added 3 commits June 21, 2018 14:44

[frontend] Add OBS Factory files

a2b7c83

We wanted to add the OBS Factory engine in this project to be part of the code of OBS. This commit only move OBS Factory Engine files into OBS.

[frontend] Adapt references to classes of OBS Factory code

f77d0e4

Some of the name where clashing with some global ones. Now we prepend the name of the module and the global accessor. Also some unused code, controller and old routes were removed.

[frontend] Fix counter_redirects

a299d1c

DavidKang force-pushed the feature/obs-factory-without-test branch from 6c3257f to a0a2a4b Compare June 21, 2018 12:46

jberry-suse reviewed Jun 21, 2018

View reviewed changes

David Kang added 4 commits June 21, 2018 21:56

[frontend] Use Backend::Api

d72d56c

We replaced the use of ActiveXML and PackageFile in favour of Backend::Api.

[ci] Update rubocop_todo.yml

5c27618

We updated rubocop_todo file to exclude all the offenses related with obs_factory.

[ci] Fix Haml Lint offenses

96337b5

[frontend] Change #staging-dashboard to be a class

97645f1

DavidKang force-pushed the feature/obs-factory-without-test branch from a0a2a4b to 97645f1 Compare June 21, 2018 19:57

coolo mentioned this pull request Jun 22, 2018

Bugfix/backend file #5183

Merged

bgeuken merged commit c32e16a into openSUSE:master Jun 22, 2018

DavidKang deleted the feature/obs-factory-without-test branch June 22, 2018 12:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate obs factory #5140

Integrate obs factory #5140

DavidKang commented Jun 14, 2018 •

edited

Loading

Ana06 left a comment

hennevogel commented Jun 14, 2018

Ana06 Jun 18, 2018

coolo Jun 20, 2018

jberry-suse Jun 20, 2018

coolo Jun 21, 2018

bgeuken Jun 22, 2018

Ana06 left a comment

bgeuken left a comment

DavidKang commented Jun 19, 2018

coolo left a comment

DavidKang commented Jun 20, 2018

bgeuken commented Jun 20, 2018

coolo commented Jun 20, 2018

bgeuken commented Jun 20, 2018

coolo commented Jun 21, 2018

coolo commented Jun 21, 2018

DavidKang commented Jun 21, 2018 •

edited

Loading

bgeuken commented Jun 21, 2018

coolo commented Jun 21, 2018

jberry-suse Jun 21, 2018

DavidKang Jun 21, 2018

Integrate obs factory #5140

Integrate obs factory #5140

Conversation

DavidKang commented Jun 14, 2018 • edited Loading

Ana06 left a comment

Choose a reason for hiding this comment

hennevogel commented Jun 14, 2018

Ana06 Jun 18, 2018

Choose a reason for hiding this comment

coolo Jun 20, 2018

Choose a reason for hiding this comment

jberry-suse Jun 20, 2018

Choose a reason for hiding this comment

coolo Jun 21, 2018

Choose a reason for hiding this comment

bgeuken Jun 22, 2018

Choose a reason for hiding this comment

Ana06 left a comment

Choose a reason for hiding this comment

bgeuken left a comment

Choose a reason for hiding this comment

DavidKang commented Jun 19, 2018

coolo left a comment

Choose a reason for hiding this comment

DavidKang commented Jun 20, 2018

bgeuken commented Jun 20, 2018

coolo commented Jun 20, 2018

bgeuken commented Jun 20, 2018

coolo commented Jun 21, 2018

coolo commented Jun 21, 2018

DavidKang commented Jun 21, 2018 • edited Loading

bgeuken commented Jun 21, 2018

coolo commented Jun 21, 2018

jberry-suse Jun 21, 2018

Choose a reason for hiding this comment

DavidKang Jun 21, 2018

Choose a reason for hiding this comment

DavidKang commented Jun 14, 2018 •

edited

Loading

DavidKang commented Jun 21, 2018 •

edited

Loading