Support balances and add warnings #483

treydock · 2019-08-30T19:20:44Z

Screenshot using default locales

Screenshot using OSC locales

ericfranz · 2019-09-03T17:05:50Z

app/models/balance.rb

@@ -0,0 +1,87 @@
+# This describes disk quota utilization for a given user and volume


Update comment to describe balance instead of disk quota. Maybe in parentheses (RUs) in comments for us on Gateways?

I would prefer that the comment refer to config.unit specifically, and call out resource units as what are used at OSC.

Shouldn't that go in README or does it belong in model header too?

ericfranz · 2019-09-03T17:10:02Z

app/models/balance.rb

+
+  def balance_object
+    return @project if @project.present?
+    @user


Can use the new presence here:

def balance_object project.presence || user end

Change made.

config/configuration_singleton.rb

README.md

MorganRodgers

I would prefer that we be explicit that config.unit can be any string value that makes sense for a given site, but I won't make that a hard requirement.

Looks good!

README.md

MorganRodgers · 2019-09-03T17:32:54Z

app/models/balance.rb

@@ -0,0 +1,87 @@
+# This describes disk quota utilization for a given user and volume


I would prefer that the comment refer to config.unit specifically, and call out resource units as what are used at OSC.

…can be any string

…ocales for units

treydock · 2019-09-03T17:56:37Z

I updated it so config key is optional and added better default locale text if no units are provided

treydock · 2019-09-03T18:01:42Z

Slightly better:

treydock · 2019-09-04T11:08:31Z

Turns out updating every 15 minutes is useless because OSC only sends charging data to database once a day, which seems logical other sites do things similarly as easier to charge for previous day than several times a day. Going to remove the refresh locale.

treydock · 2019-09-04T11:09:21Z

Actually might leave text but modify for default to daily.

treydock · 2019-09-04T12:02:16Z

One thing I added and can revert if not wanted is concept of OnDemand having timezone configured. The ActiveSupport time zone names I couldn't get from local server timezone via Time so had to configure the Rails app. I couldn't set time zone with initializer since has to be set in Application class. The absence of a timezone just sticks with UTC which is the default. Since the time used for things like balances is specific to where the server is deployed and not the user's timzone, I opted for server-side time zone configuration.

treydock · 2019-09-04T12:04:56Z

This is current look using OSC specific units and project type but default locales.

config/configuration_singleton.rb

ericfranz · 2019-09-06T18:50:24Z

We should actually revert the timezone change. In the Job Composer we use this Rails gem https://github.com/basecamp/local_time. That adds a view helper "local_time" so you can do something like this:

<%= local_time(workflow.created_at) %>

The result is the user sees something in their own time zone instead of UTC time formatted string. Thus, if I create a new job at Utah's CHPC or at OSC in the Job Composer the created at displays like this: "September 6, 2019 2:35pm" - even though the time zones are different I know as a user I created both of those 15 minutes ago since it is 2:50 PM right now.

If you want to display this string in local time that would be the most appropriate to do now.

As far as having an OOD_TIMEZONE, you could open a separate issue on the ondemand repo about the topic of displaying time in OnDemand and we could adopt a more formalized strategy that addresses drawbacks or insufficiency of using Basecamp's local_time gem.

This reverts commit 826ceb5.

This reverts commit 0095004.

treydock · 2019-09-08T14:41:21Z

Added commit that uses local_time to generate the last_update value.

ericfranz · 2019-09-09T16:39:03Z

app/views/shared/_insufficient_balance_resource.html.erb

@@ -2,7 +2,7 @@
  <%= t('dashboard.balance_warning_prefix_html', units_balance: balance.units_balance, unit: @unit) %>
  <strong><%= balance.project_type %> <%= balance.balance_object %></strong>
  <small class="pull-right hidden-xs hidden-sm">
-    <%= t('dashboard.balance_reload_message_html', last_update: balance.last_update) %>
+    <%= t('dashboard.balance_reload_message_html', last_update: local_time(balance.last_update)) %>


This could just be:

<%= t('dashboard.balance_reload_message_html', last_update: local_time(balance.updated_at)) %>

and Balance#last_update removed as Balance#last_update purpose is to make a local time human readable form of Balance#updated_at, which is what local_time(balance.updated_at) also achieves.

Change made, put format into view. I wasn't able to get newer local_time gem to work with internationalization so left hardcoded format in view.

ericfranz · 2019-09-10T18:23:37Z

app/views/shared/_insufficient_balance_resource.html.erb

@@ -0,0 +1,12 @@
+<h4>
+  <%= t('dashboard.balance_warning_prefix_html', units_balance: balance.units_balance, unit: @unit) %>


@unit is an instance variable of Balance so should this instead be:

<%= t('dashboard.balance_warning_prefix_html', units_balance: balance.units_balance, unit: balance.unit) %>

ericfranz · 2019-09-10T18:23:56Z

app/views/shared/_insufficient_balance_resource.html.erb

+</h4>
+<ul class="list-unstyled">
+  <li>
+    <%= balance.to_s %>. <%= t('dashboard.balance_additional_message', balanace_units: balance.balanace_units, unit: @unit) %>


See comment above, it seems like @unit here should actually be balance.unit

Support balances and add warnings

7c3c5b6

treydock mentioned this pull request Aug 30, 2019

Enable balances for OSC OnDemand OSC/osc-ood-config#38

Merged

ericfranz reviewed Sep 3, 2019

View reviewed changes

config/configuration_singleton.rb Show resolved Hide resolved

ericfranz reviewed Sep 3, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

ericfranz reviewed Sep 3, 2019

View reviewed changes

README.md Show resolved Hide resolved

MorganRodgers approved these changes Sep 3, 2019

View reviewed changes

treydock added 3 commits September 3, 2019 13:42

Documentation corrections

56b7599

Use presence

084778c

Document config.unit and config.project_type are used by locales and …

8265f30

…can be any string

MorganRodgers approved these changes Sep 3, 2019

View reviewed changes

Make configs for balanace JSON optional and have better default for l…

6f28033

…ocales for units

treydock added 2 commits September 3, 2019 14:00

Improve default locales

79614fa

Improve default locales - fixed duplicate balance

d125d93

treydock added 2 commits September 3, 2019 14:07

Actually return

38456dc

Still support using unit locale in balance messages

d7b7ce5

Change balance update frequency to daily

2eb023d

Add concept of timezone

0095004

treydock force-pushed the balances branch from d41a528 to 0095004 Compare September 4, 2019 12:03

ericfranz self-assigned this Sep 6, 2019

ericfranz reviewed Sep 6, 2019

View reviewed changes

config/configuration_singleton.rb Show resolved Hide resolved

Only lookup quota or balance paths once

826ceb5

treydock added 2 commits September 8, 2019 10:33

Revert "Only lookup quota or balance paths once"

588bb54

This reverts commit 826ceb5.

Revert "Add concept of timezone"

c83912e

This reverts commit 0095004.

Use local_time for balance last_update

847c2b0

treydock force-pushed the balances branch from 0a5cdc0 to 847c2b0 Compare September 9, 2019 13:34

ericfranz reviewed Sep 9, 2019

View reviewed changes

ericfranz reviewed Sep 10, 2019

View reviewed changes

treydock added 2 commits September 12, 2019 10:48

Move time logic to view

e7bac41

Proper use of unit in views

0139817

ericfranz merged commit 47cb7b3 into master Sep 18, 2019

ericfranz deleted the balances branch September 18, 2019 00:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support balances and add warnings #483

Support balances and add warnings #483

treydock commented Aug 30, 2019

ericfranz Sep 3, 2019

MorganRodgers Sep 3, 2019

treydock Sep 3, 2019

ericfranz Sep 3, 2019

treydock Sep 3, 2019

MorganRodgers left a comment

MorganRodgers Sep 3, 2019

treydock commented Sep 3, 2019

treydock commented Sep 3, 2019

treydock commented Sep 4, 2019

treydock commented Sep 4, 2019

treydock commented Sep 4, 2019

treydock commented Sep 4, 2019

ericfranz commented Sep 6, 2019

treydock commented Sep 8, 2019

ericfranz Sep 9, 2019

treydock Sep 12, 2019

ericfranz Sep 10, 2019

treydock Sep 12, 2019

ericfranz Sep 10, 2019

treydock Sep 12, 2019

		@@ -0,0 +1,87 @@
		# This describes disk quota utilization for a given user and volume

		@@ -0,0 +1,12 @@
		<h4>
		<%= t('dashboard.balance_warning_prefix_html', units_balance: balance.units_balance, unit: @unit) %>

Support balances and add warnings #483

Support balances and add warnings #483

Conversation

treydock commented Aug 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MorganRodgers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

treydock commented Sep 3, 2019

treydock commented Sep 3, 2019

treydock commented Sep 4, 2019

treydock commented Sep 4, 2019

treydock commented Sep 4, 2019

treydock commented Sep 4, 2019

ericfranz commented Sep 6, 2019

treydock commented Sep 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment