Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add migrations for new GitHub backend #632

Merged
merged 2 commits into from
Oct 10, 2018

Conversation

elyscape
Copy link
Contributor

@elyscape elyscape commented Oct 8, 2018

This pull request adds two database migrations to support the new GitHub backend, which expects that the version_url field will be in the format owner/project. The first migration converts any projects that have a URL in that field to the required format. The second migration uses the value of the homepage field to populate any projects that use the GitHub backend and have a null value for the version_url field.

@codecov-io
Copy link

codecov-io commented Oct 8, 2018

Codecov Report

Merging #632 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #632   +/-   ##
=======================================
  Coverage   90.14%   90.14%           
=======================================
  Files          56       56           
  Lines        2690     2690           
  Branches      352      352           
=======================================
  Hits         2425     2425           
  Misses        201      201           
  Partials       64       64

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59e3294...ef2767f. Read the comment docs.

Copy link
Member

@jeremycline jeremycline left a comment

Choose a reason for hiding this comment

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

Both these migrations look right to me, but I've not tested them on a database dump to make completely sure. I'll leave it up to @Zlopez to review and merge. Thanks for the PR!

@Zlopez
Copy link
Contributor

Zlopez commented Oct 9, 2018

I tried to test it, but right now I'm getting 404 when trying to receive database dump from production. So I need to wait until this will be fixed.

@Zlopez
Copy link
Contributor

Zlopez commented Oct 9, 2018

The database is frozen now and the name of the dump is different, I changed it in ansible script and run it.
There are still few projects, that doesn't have owner/repo filled after running these migrations.
Here is the list of projects, that doesn't have filled the version_url after these migrations:

select name from projects where backend='GitHub' and version_url is NULL;
 collectd-ceilometer-plugin
 releases
 django-notifications
 healpy
 client-python
 fotowall
 plymouth-kcm
 os-win
 server
 yosys
 sphinxcontrib-websupport
 LD25
 xoreos-tools
 xdg-desktop-portal-kde
 proxysql
 setroubleshoot
 Macaulay2
 libinvm-cli
 lecm
 uniconvertor
 dolphin
 dist-git-client
 opa-libhfi1verbs
 pimd
 python-tackerclient
 fc-client
 python-vitrageclient
 proj.4
 fc-admin
 libinvm-cim
 python-aodhclient
 pyprocdev
 node-csv-generate
 represent
 microversion-parse
 lxqt-admin
 stage
 murano-pkg-check
 python-ntlm3

select name from projects where backend='GitHub' and version_url='';
 xml
 revelation
 testCloud
 liblxqt-mount
 bzflag
 pgcenter
 distribution
 super_closure
 lua-term
 anymarkup-core
 pyasn1
 WizQTClient
 ssh-credentials-plugin
 python-pyhsm
 cppformat
 aeskulap
 plexus-interpolation
 com.ifedorenko.m2e.sourcelookup
 fourterm
 gnome-pomodoro
 jsonld-java-tools
 azure-sdk-for-go
 libseccomp-golang
 blivet-gui
 sxiv
 telegram-purple
 koschei
 libsearpc
 ConsoleImageViewer
 json-parse-helpfulerror
 Baka-MPlayer
 hawkular-client-go
 django-redis
 libfm
 heapster
 lxqt-about
 jackson-parent
 m2eclipse-tycho
 m2eclipse-modello
 http
 read-package-tree
 fcitx-sunpinyin
 sshrc
 libminc
 sawfish
 atomic-reactor
 memoryfilesystem
 tcmu-runner
 did
 m2eclipse-antlr
 kwplayer
 node-request-retry
 aproba
 python-jsonpath-rw-ext
 multiline
 go
 PyKMIP
 go-colortext
 mypy
 pyoptical
 lodash
 pyRiemann
 flower
 k-3d
 ironic-python-agent
 lxsession
 lxqt-qtplugin
 matrix
 openfortivpn
 mousetrap
 tosca-parser
 six
 memtailor
 go-dbus-generator
 lxqt-policykit
 xtext
 Streams
 aeson-pretty
 gettext-parser
 slingshot
 PhotoCollage
 nsdf
 heredoc
 osincli
 sys
 osin
 json-smart-v1
 pb
 qextserialport
 golang-github-shiena-ansicolor
 probing
 gucumber
 raven-go
 go-flowrate
 minitube
 lxqt-runner
 luminance-hdr
 cave9
 foobnix
 asn1-ber
 internal
 ratelimit
 floats
 rhythmbox-ampache
 doxy2man
 rkt
 sorted-object
 ed25519
 resumable
 swift
 goamz
 go-ceph
 qlipper
 flacon
 sddm-themes
 SMB
 formatio
 rpdb
 Slic3r
 PyPDF2
 bugsnag-go
 encoding
 gnome-mpv
 qxkb
 realize-package-specifier
 Hystrix
 reaver-wps
 Zake
 openshift-sdn
 iksemel
 common
 jwt-go
 yaml-mode
 tycho-pomless
 go-homedir
 jWalterWeatherman
 package-query
 pq
 properties
 viper
 pkcs11
 external-monitor-job-plugin
 lxqt-notificationd
 lognormalizer
 lxqt-sudo
 gotuf
 CPUFreqUtility
 osbs-client
 plexus-io
 vtable-dumper
 etag
 when
 gdalcpp
 stun
 ES6-Promises
 pyjavascriptcore
 go-iptables
 doubanfm-qt
 freshplayerplugin
 wsgi-intercept
 bcloud
 freight-tools
 fs
 custodia
 deepin-utils
 polyglot
 application
 yaourt
 forms
 wiki
 cache_tab
 XwareDesktop
 go-cleanhttp
 PointDownload
 cdogs-sdl
 errwrap
 xpybutil
 oglematchers
 PETPVC
 cmark
 go-immutable-radix
 datadog-go
 mongo-lint
 go-memdb
 system-config-repo
 mathicgb
 security
 ClipIt
 go-sqlmock
 di
 fbpanel
 smtpping
 profile
 ldapserver
 lxqt-config
 pcmanfm-qt
 lxterminal
 tracy
 OpenCobolIDE
 sysreporter
 caching
 lxqt-globalkeys
 iwyu
 arora
 openstv
 mathic
 system-config-printer
 p1_utils
 tmuxinator
 screenie
 bastet
 mixxx
 DNSSEC4J
 m2eclipse-sisu
 compress
 m2eclipse-buildhelper
 m2eclipse-cxf
 m2eclipse-mavenarchiver
 m2eclipse-plexus
 database
 bumpversion
 sezpoz
 2048-cli
 commoner
 plexus-digest
 joda-convert
 acd_cli
 get-stdin
 htrace
 cronolog
 prozilla
 shflags
 tar-split
 asn1-ber
 skopeo
 go-sqlite3
 moss
 tycho-support
 repo_manager
 q
 swarm
 python-lazy-object-proxy
 libsmbclient-php
 tls
 reflection
 plexus-compiler
 pyxid
 ldap
 gorm
 percol
 lxqt-panel
 ssh-slaves-plugin
 jsonld-java
 maven-stapler-plugin.git
 natsort
 qmpdclient
 php-cssjanus
 lollypop
 winetricks
 lxqt-powermanagement
 fgrun
 gradunwarp
 junit-plugin
 instack
 install
 lxqt-session
 html2text
 credentials-plugin
 ant-plugin
 bibletime
 target-isns
 valabind
 TarStreamer
 unclutter
 gimagereader
 luafun
 gnome-shell-extension-openweather
 roboto-fontface-bower
 shadowsocks
 obs-studio
 shadowsocks-qt5
 nvme-cli
 runc
 OpenCC
 deepin-ui
 com.ifedorenko.m2e.mavendev
 concurrentlinkedhashmap
 libwhirlpool
 puzzle-master
 obconf
 sunpinyin
 openambit
 javadoc-plugin
 riffle
 tycho-support
 lpf
 compton
 protozero
 QtAV
 specs
 plexus-classworlds
 aws-sdk-go
 xdotool
 mysql
 go-lib
 source-to-image
 psi-plus
 deepin-music
 deepin-screenshot
 sqlitebrowser
 deepin-movie
 chocolate-doom
 constant-pool-scanner
 edb
 Assert
 cockpit
 elementary-xfce-icon-theme
 DiffusionKurtosisFit
 berusky
 dnf-plugin-system-upgrade
 fedora-review-plugin-java
 figlet
 dbus-factory
 graph
 fsnotify
 jose2go
 golang-github-spf13-cast
 lrbd
 msgpuck
 node-mkfiletree
 nodeenv
 node-error-ex
 openid4java-team-extension
 openid-plugin
 lightncandy
 pingus
 libfm-qt
 proxy
 p1_sip
 mailer-plugin
 os-maven-plugin
 oss-parents
 mail
 PackageKit
 pnmixer
 handlers
 sessions
 positional
 robot-loader
 shell
 moonplayer
 stapler-adjunct-timeline
 volti
 stringprep
 guice
 libbytesize
 nqp
 beanbag
 koji-containerbuild
 machine
 panicwrap
 pinta
 handlers
 sessions
 positional
 robot-loader
 shell
 moonplayer
 stapler-adjunct-timeline
 volti
 stringprep
 guice
 libbytesize
 nqp
 beanbag
 koji-containerbuild
 machine
 panicwrap
 pinta
 lxqt-openssh-askpass
 flysystem
 colorhug-client
 plexus-resources
 synce-trayicon
 libdkimpp
 londonlaw
 tesla-pom
 qcomicbook
 resource-operations
 sphinxcontrib-fulltoc
 plexus-component-factories
 musicbox
 php-compatinfo-db
 go-units
 strip-indent
 libQtShadowsocks
 takari-pom
 semver
 p1_yaml
 python-unicodecsv
 os-tmpdir
 sql-parser
 url-parse-lax
 xsensors
 basic-auth
 spdx-expression-parse.js.git
 m2eclipse-egit
 plexus-components
 ssr
 pragha
 openav
 qtstyleplugins

It will be enough, if you could just add also the projects with version_url='' to your script. And do a trim of homepage for any whitespace before trimming it for slashes. Then I think it will catch all projects with GitHub backend.

Copy link
Contributor

@Zlopez Zlopez left a comment

Choose a reason for hiding this comment

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

Your PR looks good I have only few small changes that should help catch any missing project with GitHub backend.


def downgrade():
"""No-op, as empty version_url wouldn't have worked before anyway."""
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to raise NotImplementedError

SET version_url=trim(substr(homepage, 20), '/')
WHERE backend = 'GitHub'
AND homepage LIKE 'https://github.com/%'
AND version_url IS NULL
Copy link
Contributor

Choose a reason for hiding this comment

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

Add OR version_url=''

SET version_url=trim(substr(homepage, 19), '/')
WHERE backend = 'GitHub'
AND homepage LIKE 'http://github.com/%'
AND version_url IS NULL
Copy link
Contributor

Choose a reason for hiding this comment

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

Add OR version_url=''

Copy link
Contributor

@Zlopez Zlopez left a comment

Choose a reason for hiding this comment

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

One more small change.



def downgrade():
"""Convert GitHub owner/project to URL to work with old GitHub backend."""
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't necessery, the old backend can work with owner/repo in version_url.

@Zlopez
Copy link
Contributor

Zlopez commented Oct 9, 2018

I found out, I tried your migration script on wrong db dump.

So I started again and found out, there is issue with the new dump, because it's missing all user related tables.
I hope this will be resolved soon. I created a ticket for it in fedora infra: https://pagure.io/fedora-infrastructure/issue/7298

The new GitHub backend expects that version_url will be an owner/project
pair rather than a URL.

Signed-off-by: Eli Young <[email protected]>
This uses the homepage field to populate any null version_url fields on
projects that use the GitHub backend.

Signed-off-by: Eli Young <[email protected]>
@elyscape
Copy link
Contributor Author

elyscape commented Oct 9, 2018

@Zlopez Comments addressed.

Copy link
Contributor

@Zlopez Zlopez left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot merged commit 0ecdf94 into fedora-infra:master Oct 10, 2018
@elyscape elyscape deleted the github-migration branch October 10, 2018 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants