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

CORE-1889 Tapis v3 Migration #270

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

psarando
Copy link
Member

This PR will rename everything from agave to tapis and add updates for cyverse-de/mescal#25 and cyverse-de/common-swagger-api#85, which must be merged first before this PR can be merged.

@@ -73,8 +66,7 @@

(defn store-access-token
"Stores information about an OAuth access token in the database."
[api-name username {:keys [token-type expires-at refresh-token access-token]}]
(validate-token-type token-type)
Copy link
Member Author

Choose a reason for hiding this comment

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

Tapis no longer returns a token-type field.

@@ -365,4 +365,5 @@
(.hasAppPermission (util/get-apps-client clients system-id) username system-id app-id required-level))

(supportsJobSharing [_ job-step]
(.supportsJobSharing (util/apps-client-for-job-step clients job-step) job-step)))
(and (not= (:job_type job-step) jp/agave-job-type)
(.supportsJobSharing (util/apps-client-for-job-step clients job-step) job-step))))
Copy link
Member Author

Choose a reason for hiding this comment

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

The changes in this namespace are only for allowing users to still list their old Agave jobs, even if they can't relaunch them anymore.

Maybe it would be better to re-add an Agave type that handles the logic in these methods, and throws an error for the rest? 🤷

Copy link
Member

Choose a reason for hiding this comment

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

That seems pretty clean to me, but this is also understandable, so I'm open to leaving it as is.

@@ -51,8 +52,7 @@
"FolderInput" "collection"
"MultiFileSelector" "many"})

(def input-types
(set (keys input-multiplicities)))
(def input-types am/param-ds-input-types)
Copy link
Member Author

Choose a reason for hiding this comment

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

The HPC FileFolderInput type was missing from this before, so inputs were not repopulating in the UI relaunch forms correctly.

(log/info (str "received a status update for Agave job " external-id ": status = " status))
(apps/update-job-status job-id external-id status (first (remove string/blank? [end-time last-updated]))))
(service/assert-valid type "no status provided")
(let [{:keys [jobUuid]} (service/parse-json data)
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why once again, but this data field is JSON encoded as a string, so we have to parse it here in order to get the Tapis job ID.

Copy link
Member

@slr71 slr71 left a comment

Choose a reason for hiding this comment

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

Looks good to me! :shipit:

@@ -365,4 +365,5 @@
(.hasAppPermission (util/get-apps-client clients system-id) username system-id app-id required-level))

(supportsJobSharing [_ job-step]
(.supportsJobSharing (util/apps-client-for-job-step clients job-step) job-step)))
(and (not= (:job_type job-step) jp/agave-job-type)
(.supportsJobSharing (util/apps-client-for-job-step clients job-step) job-step))))
Copy link
Member

Choose a reason for hiding this comment

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

That seems pretty clean to me, but this is also understandable, so I'm open to leaving it as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants