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

dataset info: present decoded IDs for admins and add peek #5006

Merged
merged 1 commit into from Nov 15, 2017

Conversation

martenson
Copy link
Member

ping @natefoo

@natefoo
Copy link
Member

natefoo commented Nov 14, 2017

Record time from request to PR, thank you @martenson!

@@ -174,11 +174,26 @@
%if job:
<tr><td>Tool Exit Code:</td><td>${ job.exit_code | h }</td></tr>
%endif
<tr><td>History Content API ID:</td><td>${encoded_hda_id}</td></tr>
<tr><td>History Content API ID:</td>
Copy link
Member

Choose a reason for hiding this comment

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

Should we also change the name to indicate that it is a decoded id for the admin?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought this is clear for people that are actually using IDs for anything. What would you suggest?

Copy link
Member

Choose a reason for hiding this comment

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

Simply adding (decoded)? If this is obvious, all good.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had it there, but @natefoo called it superfluous.

Copy link
Member

Choose a reason for hiding this comment

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

ok :)

@mvdbeek
Copy link
Member

mvdbeek commented Nov 14, 2017

I often use this as a starting point to interact with the API. Can we toggle this instead ?

@martenson
Copy link
Member Author

@mvdbeek both are present

@mvdbeek
Copy link
Member

mvdbeek commented Nov 14, 2017

D'oh. 👍 (I did look at the code this time).

@jmchilton
Copy link
Member

Do we have a plan for replicating this via the API longer term? An admin only api endpoint to decode IDS maybe?

@martenson
Copy link
Member Author

@jmchilton I would enhance the dict for admin using the same endpoint, is that a nono now?

@jmchilton
Copy link
Member

@martenson That would be fine with me I think - I just wanted to know what the plan is.

@martenson
Copy link
Member Author

@jmchilton I will implement that in a different PR

@jmchilton jmchilton merged commit 482ff77 into galaxyproject:dev Nov 15, 2017
@martenson martenson deleted the decode_for_admins branch November 27, 2017 04:12
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.

None yet

5 participants