558 access token for private repositories #1136
Conversation
| @@ -1,7 +1,7 @@ | |||
| class Ability | |||
| include CanCan::Ability | |||
|
|
|||
| def initialize(user) | |||
| def initialize(user, access_token) | |||
There was a problem hiding this comment.
Metrics/CyclomaticComplexity: Cyclomatic complexity for initialize is too high. [14/6]
Metrics/MethodLength: Method has too many lines. [102/10]
Metrics/PerceivedComplexity: Perceived complexity for initialize is too high. [17/7]
There was a problem hiding this comment.
I think we can ignore this because the ability file is a special case.
eugenk
force-pushed
the
558-access_token_for_private_repositories
branch
2 times, most recently
from
19a0376
to
23c5ed5
Compare
| a | ||
| end | ||
|
|
||
| protected |
There was a problem hiding this comment.
Lint/UselessAccessModifier: Useless protected access modifier.
eugenk
force-pushed
the
558-access_token_for_private_repositories
branch
2 times, most recently
from
b5796fe
to
fbac705
Compare
| Time.now.strftime('%Y-%m-%d-%H-%M-%S-%6N') | ||
| ].join('|') | ||
| AccessToken.new( | ||
| { repository: repository, |
There was a problem hiding this comment.
Style/SpaceInsideHashLiteralBraces: Space inside { detected.
eugenk
force-pushed
the
558-access_token_for_private_repositories
branch
from
fbac705
to
a60ebc7
Compare
|
Why do we use/are we supposed to use the crontab? Why not work directly with sidekiq (e.g. by using a |
|
Good point. Is there a sidekiq feature for waiting before performing a task? By the way, this should actually be done by sidetiq, but somehow sidetiq depends on cron in our setup. I had a private rails4 project where sidetiq did work without cron, but I can't tell why ontohub doesn't. |
|
I don't know either why we have such problems with sidetiq, maybe we should investigate that. With regard to the 'waiting': Take a look at the TimeoutWorker, it is using this feature. |
| :kind => resource.entities.groups_by_kind.first.kind) | ||
| if resource.distributed? | ||
| redirect_to repository_ontology_children_path(parent, resource, | ||
| params_to_pass_on_redirect.merge( |
There was a problem hiding this comment.
Please do not break the line after the opening parentheses of a method-call.
eugenk
force-pushed
the
558-access_token_for_private_repositories
branch
from
4853307
to
a5046cf
Compare
|
This is now fully adjusted to the changes of interfacing with Hets. It is open for review again. |
|
Oh and I rebased this branch on staging. In this process, the The first new commit is: |
|
|
||
| def params_to_pass_on_redirect | ||
| new_params = {} | ||
| %i(access-token).each { |key| new_params[key] = params[key] if params[key] } |
There was a problem hiding this comment.
Right. select is better :D
| @@ -4,6 +4,24 @@ module Repository::Access | |||
| OPTIONS = %w[public_r public_rw private_r private_rw] | |||
| DEFAULT_OPTION = OPTIONS[0] | |||
|
|
|||
| def self.as_read_only(access) | |||
| access.split('_').first + '_r' | |||
There was a problem hiding this comment.
how about using a limit to split (e.g. ...split('_', 2)) so optimizations can kick in? I know it's probably just a short string, but why not ;)
|
👍 |
…positories 558 access token for private repositories
|
Changes Unknown when pulling 3364a2a on 558-access_token_for_private_repositories into * on staging*. |
This shall fix #558. As soon as hets (spechub/Hets#1422) can handle it completely, this should work. Now, it is working for non-owl files whose file type can be detected by hets' magic.
After merging, don't forget to put this line into the crontab for the expired tokens to be deleted daily: